netdev - Re: [PATCH iproute2-next v2] tc flower: use right ethertype in icmp/arp parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Nov 2020 14:13:51 +0200
From:   Roi Dayan <roid@...dia.com>
To:     David Ahern <dsahern@...il.com>,
        Zahari Doychev <zahari.doychev@...ux.com>,
        <netdev@...r.kernel.org>
CC:     <simon.horman@...ronome.com>, <jhs@...atatu.com>,
        <jianbol@...lanox.com>
Subject: Re: [PATCH iproute2-next v2] tc flower: use right ethertype in
 icmp/arp parsing



On 2020-11-24 11:39 AM, Roi Dayan wrote:
> 
> 
> On 2020-11-14 5:12 AM, David Ahern wrote:
>> On 11/10/20 12:53 AM, Zahari Doychev wrote:
>>> Currently the icmp and arp parsing functions are called with incorrect
>>> ethtype in case of vlan or cvlan filter options. In this case either
>>> cvlan_ethtype or vlan_ethtype has to be used. The ethtype is now updated
>>> each time a vlan ethtype is matched during parsing.
>>>
>>> Signed-off-by: Zahari Doychev <zahari.doychev@...ux.com>
>>> ---
>>>   tc/f_flower.c | 52 +++++++++++++++++++++++----------------------------
>>>   1 file changed, 23 insertions(+), 29 deletions(-)
>>>
>>
>> Thanks, looks much better.
>>
>> applied to iproute2-next.
>>
> 
> Hi,
> 
> I didn't debug yet but with this commit I am failing to add a tc
> rule I always could before. also the error msg doesn't make sense.
> 
> Example:
> 
> # tc filter add dev enp8s0f0 protocol 802.1Q parent ffff: prio 1 flower\
>   skip_hw dst_mac e4:11:22:11:4a:51 src_mac e4:11:22:11:4a:50\
>   vlan_ethtype 0x800 vlan_id 100 vlan_prio 0 action vlan pop action\
>   mirred egress redirect dev enp8s0f0_0
> 
> 
> Can't set "vlan_id" if ethertype isn't 802.1Q or 802.1AD
> 
> 
> I used protocol 802.1Q and vlan_ethtype 0x800.
> am i missing something? the rule should look different now?
> 
> Thanks,
> Roi


Hi,

I debugged this and it break vlan rules.
The issue is from this part of the change


@@ -1464,6 +1464,8 @@ static int flower_parse_opt(struct filter_util 
*qu, char *handle,
                                                  &vlan_ethtype, n);
                         if (ret < 0)
                                 return -1;
+                       /* get new ethtype for later parsing  */
+                       eth_type = vlan_ethtype;



Now params vlan_id, vlan_prio check if eth_type is vlan but it's not.
it's 0x0800 as the example as it was set to the vlan_ethtype.

Need to continue check the outer, so tc_proto.
i'll prep a fix commit for review.


Thanks,
Roi