Dan Kenigsberg has posted comments on this change. Change subject: net: Support VLAN hardware offloading in hostQOS ......................................................................
Patch Set 1: Code-Review-1 (5 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() ? 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 TestFilters.test_filters 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 live with this. But I think that using split('(', 1) is a bit safer. 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 comment. Line 151: return data Line 152: Line 153: Line 154: _parse_match_ip = _parser.parse_skip_line # Unimplemented, skip line Line 174: if token in ('eq', 'lt', 'gt'): Line 175: data['relation'] = token Line 176: data['value'] = next(tokens).strip(')') Line 177: elif token == 'mask': Line 178: data['mask'] = _parser.parse_str(tokens) again, please add a protective final "else". Line 179: return data Line 180: Line 181: Line 182: def _parse_action(tokens): -- 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 <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
