Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH net] net: ethtool: Allow matching on vlan CFI bit
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>,
        davem@davemloft.net, Pablo Neira Ayuso <pablo@netfilter.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jiri Pirko <jiri@mellanox.com>,
        Jakub Kicinski <jakub.kicinski@netronome.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Antoine Tenart <antoine.tenart@bootlin.com>,
        thomas.petazzoni@bootlin.com,
        =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= <mirq-linux@rere.qmqm.pl>
References: <20190611155456.15360-1-maxime.chevallier@bootlin.com>
From:   Toshiaki Makita <toshiaki.makita1@gmail.com>
Message-ID: <e4017b1b-56ac-7853-ea81-f89a7f5c9890@gmail.com>
Date:   Wed, 12 Jun 2019 15:31:54 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <20190611155456.15360-1-maxime.chevallier@bootlin.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2019/06/12 0:54, Maxime Chevallier wrote:
> Using ethtool, users can specify a classification action matching on the
> full vlan tag, which includes the CFI bit.
> 
> However, when converting the ethool_flow_spec to a flow_rule, we use
> dissector keys to represent the matching patterns.
> 
> Since the vlan dissector key doesn't include the CFI bit, this
> information was silently discarded when translating the ethtool
> flow spec in to a flow_rule.
> 
> This commit adds the CFI bit into the vlan dissector key, and allows
> propagating the information to the driver when parsing the ethtool flow
> spec.
> 
> Fixes: eca4205f9ec3 ("ethtool: add ethtool_rx_flow_spec to flow_rule structure translator")
> Reported-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> Hi all,
> 
> Although this prevents information to be silently discarded when parsing
> an ethtool_flow_spec, this information doesn't seem to be used by any
> driver that converts an ethtool_flow_spec to a flow_rule, hence I'm not
> sure this is suitable for -net.
> 
> Thanks,
> 
> Maxime
> 
>   include/net/flow_dissector.h | 1 +
>   net/core/ethtool.c           | 5 +++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index 7c5a8d9a8d2a..9d2e395c6568 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -46,6 +46,7 @@ struct flow_dissector_key_tags {
>   
>   struct flow_dissector_key_vlan {
>   	u16	vlan_id:12,
> +		vlan_cfi:1,

Current IEEE 802.1Q defines this bit as DEI not CFI, so IMO this should be
vlan_dei.

Toshiaki Makita