Change in vdsm[ovirt-3.6]: net: Support VLAN hardware offloading in hostQOS

ibarkan at redhat.com ibarkan at redhat.com
Mon Oct 19 10:13:30 UTC 2015


Ido Barkan has posted comments on this change.

Change subject: net: Support VLAN hardware offloading in hostQOS
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/47443/1/vdsm/network/configurators/qos.py
File vdsm/network/configurators/qos.py:

Line 129: 
Line 130: 
Line 131: def _qdisc_conf_out(dev, root_qdisc_handle, vlan_tag, class_id, qos):
Line 132:     """Adds the traffic class and filtering to the current hfsc qdisc"""
Line 133:     flow_id = _ROOT_QDISC_HANDLE + class_id
> update test_qdiscs() ?
it is so lame, that it does not justify the amount of work it would take to modify it to properly cover my changes. most of them are in qos.py in this logic is not covered here.
however, TestFilters might be worth a change.
Line 134: 
Line 135:     def filt_flow_id(filt, kind):
Line 136:         return filt.get(kind, {}).get('flowid')
Line 137: 


https://gerrit.ovirt.org/#/c/47443/1/vdsm/network/tc/filter.py
File vdsm/network/tc/filter.py:

Line 131: 
Line 132: 
Line 133: def _parse_ematch(tokens):
Line 134:     """Parses tokens describing a raw ematch, (see man tc-ematch) e.g.,
Line 135:     'meta(vlan mask 0x00000000 eq 16)' into a data dictionary.
> please add a filter like this one to
Done
Line 136:     currently only a single 'meta' module predicate is supported"""
Line 137:     data = {}
Line 138:     for token in tokens:
Line 139:         if token == _parser.LINE_DELIMITER:  # line break


Line 142:             data[token] = _parser.parse_str(tokens)
Line 143:         elif token == 'flowid':
Line 144:             data['flowid'] = _parser.parse_str(tokens)
Line 145:         elif '(' in token:
Line 146:             module, first_arg = token.split('(')
> Your Compilation 101 teacher now hates you for not tokenizing '('. I can li
Done
Line 147:             if module != 'meta':
Line 148:                 _parser.parse_skip_line(tokens)
Line 149:             data['module'] = module
Line 150:             data.update(_parse_ematch_match(first_arg, tokens))


Line 146:             module, first_arg = token.split('(')
Line 147:             if module != 'meta':
Line 148:                 _parser.parse_skip_line(tokens)
Line 149:             data['module'] = module
Line 150:             data.update(_parse_ematch_match(first_arg, tokens))
> please add a final "else" clause. With either a log or an apologetic commen
well, I can add a per-unknown-token log but it will not say much. I prefer fail here silently as we would get a partially parsed line rather then failing.
Line 151:     return data
Line 152: 
Line 153: 
Line 154: _parse_match_ip = _parser.parse_skip_line  # Unimplemented, skip line


-- 
To view, visit https://gerrit.ovirt.org/47443
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I667a9f38f4314da309685f6ba247f705a2e9c23e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list