On 10/19/20 5:47 AM, Zahari Doychev wrote: > Currently the icmp and arp prsing functions are called with inccorect > ethtype in case of vlan or cvlan filter options. In this case either > cvlan_ethtype or vlan_ethtype has to be used. > > Signed-off-by: Zahari Doychev <zahari.doyc...@linux.com> > --- > tc/f_flower.c | 43 ++++++++++++++++++++++++++----------------- > 1 file changed, 26 insertions(+), 17 deletions(-) > > diff --git a/tc/f_flower.c b/tc/f_flower.c > index 00c919fd..dd9f3446 100644 > --- a/tc/f_flower.c > +++ b/tc/f_flower.c > @@ -1712,7 +1712,10 @@ static int flower_parse_opt(struct filter_util *qu, > char *handle, > } > } else if (matches(*argv, "type") == 0) { > NEXT_ARG(); > - ret = flower_parse_icmp(*argv, eth_type, ip_proto, > + ret = flower_parse_icmp(*argv, cvlan_ethtype ? > + cvlan_ethtype : vlan_ethtype ? > + vlan_ethtype : eth_type, > + ip_proto,
looks correct to me, but would like confirmation of the intent from Simon. Also, I am not a fan of the readability of that coding style. Rather than repeat that expression multiple times, make a short helper to return the relevant eth type and use a temp variable for it. You should also comment that relevant eth type changes as arguments are parsed. Thanks,