From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamal Hadi Salim Subject: Re: [Patch net] net_sched: refetch skb protocol for each filter Date: Sat, 12 Jan 2019 07:23:05 -0500 Message-ID: References: <20190112025542.397-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Martin Olsson , Jiri Pirko , Lucas Bates To: Cong Wang , netdev@vger.kernel.org Return-path: Received: from mail-it1-f195.google.com ([209.85.166.195]:55373 "EHLO mail-it1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725824AbfALMXI (ORCPT ); Sat, 12 Jan 2019 07:23:08 -0500 Received: by mail-it1-f195.google.com with SMTP id m62so6846018ith.5 for ; Sat, 12 Jan 2019 04:23:08 -0800 (PST) In-Reply-To: <20190112025542.397-1-xiyou.wangcong@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2019-01-11 9:55 p.m., Cong Wang wrote: > Martin reported a set of filters don't work after changing > from reclassify to continue. Looking into the code, it > looks like skb protocol is not always fetched for each > iteration of the filters. But, as demonstrated by Martin, > TC actions could modify skb->protocol, for example act_vlan, > this means we have to refetch skb protocol in each iteration, > rather than using the one we fetch in the beginning of the loop. > > This bug is _not_ introduced by commit 3b3ae880266d > ("net: sched: consolidate tc_classify{,_compat}"), technically, > if act_vlan is the only action that modifies skb protocol, then > it is commit c7e2b9689ef8 ("sched: introduce vlan action") which > introduced this bug. > +Cc Lucas Do we have a test case for a setup like this in tdc? i.e incoming tagged and then vlan popped by action? Otherwise a test with IFE which resets the ethertype would be sufficient i.e just something that will messup with skb->protocol. Cong, I am a little worried about the impact of this change. Smells more like it has to do with Vlan action or related issues than with reclassifying. Martin, did this setup use to work before? If yes, rough idea of which kernels? cheers, jamal