| Message ID | 628ade92d458e62f9471911d3cf8f3b193212eaa.1585331173.git.petrm@mellanox.com |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | Support pedit of ip6 dsfield | expand |
On Fri, 27 Mar 2020 20:55:08 +0300 Petr Machata <petrm@mellanox.com> wrote: > Support keywords dsfield, traffic_class and tos in the IPv6 context. > > Signed-off-by: Petr Machata <petrm@mellanox.com> > --- > man/man8/tc-pedit.8 | 14 ++++++++++++-- > tc/p_ip6.c | 16 ++++++++++++++++ > 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8 > index bbd725c4..b44b0263 100644 > --- a/man/man8/tc-pedit.8 > +++ b/man/man8/tc-pedit.8 > @@ -60,8 +60,8 @@ pedit - generic packet editor action > > .ti -8 > .IR IP6HDR_FIELD " := { " > -.BR src " | " dst " | " flow_lbl " | " payload_len " | " nexthdr " |" > -.BR hoplimit " }" > +.BR src " | " dst " | " tos " | " dsfield " | " traffic_class " | " > +.BR flow_lbl " | " payload_len " | " nexthdr " |" hoplimit " }" > > .ti -8 > .IR TCPHDR_FIELD " := { " > @@ -228,6 +228,16 @@ are: > .B src > .TQ > .B dst > +.TP > +.B tos > +.TQ > +.B dsfield > +.TQ > +.B traffic_class > +Traffic Class, an 8-bit quantity. Because the field is shifted by 4 bits, > +it is necessary to specify the full 16-bit halfword, with the actual > +dsfield value sandwiched between 4-bit zeroes. > +.TP > .TQ > .B flow_lbl > .TQ > diff --git a/tc/p_ip6.c b/tc/p_ip6.c > index 7cc7997b..b6fe81f5 100644 > --- a/tc/p_ip6.c > +++ b/tc/p_ip6.c > @@ -56,6 +56,22 @@ parse_ip6(int *argc_p, char ***argv_p, > res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey); > goto done; > } > + if (strcmp(*argv, "traffic_class") == 0 || > + strcmp(*argv, "tos") == 0 || > + strcmp(*argv, "dsfield") == 0) { > + NEXT_ARG(); > + tkey->off = 1; > + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey); > + > + /* Shift the field by 4 bits on success. */ > + if (!res) { > + int nkeys = sel->sel.nkeys; > + struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1]; > + key->mask = htonl(ntohl(key->mask) << 4 | 0xf); > + key->val = htonl(ntohl(key->val) << 4); > + } > + goto done; > + } Why in the middle of the list? Why three aliases for the same value? Since this is new code choose one and make it match what IPv6 standard calls that field.
Stephen Hemminger <stephen@networkplumber.org> writes: >> diff --git a/tc/p_ip6.c b/tc/p_ip6.c >> index 7cc7997b..b6fe81f5 100644 >> --- a/tc/p_ip6.c >> +++ b/tc/p_ip6.c >> @@ -56,6 +56,22 @@ parse_ip6(int *argc_p, char ***argv_p, >> res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey); >> goto done; >> } >> + if (strcmp(*argv, "traffic_class") == 0 || >> + strcmp(*argv, "tos") == 0 || >> + strcmp(*argv, "dsfield") == 0) { >> + NEXT_ARG(); >> + tkey->off = 1; >> + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey); >> + >> + /* Shift the field by 4 bits on success. */ >> + if (!res) { >> + int nkeys = sel->sel.nkeys; >> + struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1]; >> + key->mask = htonl(ntohl(key->mask) << 4 | 0xf); >> + key->val = htonl(ntohl(key->val) << 4); >> + } >> + goto done; >> + } > Why in the middle of the list? Because that's the order IPv4 code does them. > Why three aliases for the same value? > Since this is new code choose one and make it match what IPv6 standard > calls that field. TOS because flower also calls it TOS, even if it's the IPv6 field. dsfield, because the IPv4 pedit also accepts this. I'm fine with just accepting traffic_class though.
On 3/30/20 2:32 AM, Petr Machata wrote: > > Stephen Hemminger <stephen@networkplumber.org> writes: > >>> diff --git a/tc/p_ip6.c b/tc/p_ip6.c >>> index 7cc7997b..b6fe81f5 100644 >>> --- a/tc/p_ip6.c >>> +++ b/tc/p_ip6.c >>> @@ -56,6 +56,22 @@ parse_ip6(int *argc_p, char ***argv_p, >>> res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey); >>> goto done; >>> } >>> + if (strcmp(*argv, "traffic_class") == 0 || >>> + strcmp(*argv, "tos") == 0 || >>> + strcmp(*argv, "dsfield") == 0) { >>> + NEXT_ARG(); >>> + tkey->off = 1; >>> + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey); >>> + >>> + /* Shift the field by 4 bits on success. */ >>> + if (!res) { >>> + int nkeys = sel->sel.nkeys; >>> + struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1]; >>> + key->mask = htonl(ntohl(key->mask) << 4 | 0xf); >>> + key->val = htonl(ntohl(key->val) << 4); >>> + } >>> + goto done; >>> + } >> Why in the middle of the list? > > Because that's the order IPv4 code does them. neither parse function uses matches() so the order should not matter. > >> Why three aliases for the same value? >> Since this is new code choose one and make it match what IPv6 standard >> calls that field. > > TOS because flower also calls it TOS, even if it's the IPv6 field. > dsfield, because the IPv4 pedit also accepts this. I'm fine with just > accepting traffic_class though. > that's probably the right thing to do since this is ipv6 related
David Ahern <dsahern@gmail.com> writes: > On 3/30/20 2:32 AM, Petr Machata wrote: >> >> Stephen Hemminger <stephen@networkplumber.org> writes: >> >>>> diff --git a/tc/p_ip6.c b/tc/p_ip6.c >>>> index 7cc7997b..b6fe81f5 100644 >>>> --- a/tc/p_ip6.c >>>> +++ b/tc/p_ip6.c >>>> @@ -56,6 +56,22 @@ parse_ip6(int *argc_p, char ***argv_p, >>>> res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey); >>>> goto done; >>>> } >>>> + if (strcmp(*argv, "traffic_class") == 0 || >>>> + strcmp(*argv, "tos") == 0 || >>>> + strcmp(*argv, "dsfield") == 0) { >>>> + NEXT_ARG(); >>>> + tkey->off = 1; >>>> + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey); >>>> + >>>> + /* Shift the field by 4 bits on success. */ >>>> + if (!res) { >>>> + int nkeys = sel->sel.nkeys; >>>> + struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1]; >>>> + key->mask = htonl(ntohl(key->mask) << 4 | 0xf); >>>> + key->val = htonl(ntohl(key->val) << 4); >>>> + } >>>> + goto done; >>>> + } >>> Why in the middle of the list? >> >> Because that's the order IPv4 code does them. > > neither parse function uses matches() so the order should not matter. It was purely a consistency thing. So you both seem to imply I should move it to the end, so I'll do that in v2. >> >>> Why three aliases for the same value? >>> Since this is new code choose one and make it match what IPv6 standard >>> calls that field. >> >> TOS because flower also calls it TOS, even if it's the IPv6 field. >> dsfield, because the IPv4 pedit also accepts this. I'm fine with just >> accepting traffic_class though. >> > > that's probably the right thing to do since this is ipv6 related All right, I'll send v2 with this fix.
diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8 index bbd725c4..b44b0263 100644 --- a/man/man8/tc-pedit.8 +++ b/man/man8/tc-pedit.8 @@ -60,8 +60,8 @@ pedit - generic packet editor action .ti -8 .IR IP6HDR_FIELD " := { " -.BR src " | " dst " | " flow_lbl " | " payload_len " | " nexthdr " |" -.BR hoplimit " }" +.BR src " | " dst " | " tos " | " dsfield " | " traffic_class " | " +.BR flow_lbl " | " payload_len " | " nexthdr " |" hoplimit " }" .ti -8 .IR TCPHDR_FIELD " := { " @@ -228,6 +228,16 @@ are: .B src .TQ .B dst +.TP +.B tos +.TQ +.B dsfield +.TQ +.B traffic_class +Traffic Class, an 8-bit quantity. Because the field is shifted by 4 bits, +it is necessary to specify the full 16-bit halfword, with the actual +dsfield value sandwiched between 4-bit zeroes. +.TP .TQ .B flow_lbl .TQ diff --git a/tc/p_ip6.c b/tc/p_ip6.c index 7cc7997b..b6fe81f5 100644 --- a/tc/p_ip6.c +++ b/tc/p_ip6.c @@ -56,6 +56,22 @@ parse_ip6(int *argc_p, char ***argv_p, res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey); goto done; } + if (strcmp(*argv, "traffic_class") == 0 || + strcmp(*argv, "tos") == 0 || + strcmp(*argv, "dsfield") == 0) { + NEXT_ARG(); + tkey->off = 1; + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey); + + /* Shift the field by 4 bits on success. */ + if (!res) { + int nkeys = sel->sel.nkeys; + struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1]; + key->mask = htonl(ntohl(key->mask) << 4 | 0xf); + key->val = htonl(ntohl(key->val) << 4); + } + goto done; + } if (strcmp(*argv, "payload_len") == 0) { NEXT_ARG(); tkey->off = 4;
Support keywords dsfield, traffic_class and tos in the IPv6 context. Signed-off-by: Petr Machata <petrm@mellanox.com> --- man/man8/tc-pedit.8 | 14 ++++++++++++-- tc/p_ip6.c | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-)