All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: ethtool: Allow matching on vlan CFI bit
@ 2019-06-11 15:54 Maxime Chevallier
  2019-06-11 18:27 ` Jakub Kicinski
  2019-06-12  6:31 ` Toshiaki Makita
  0 siblings, 2 replies; 3+ messages in thread
From: Maxime Chevallier @ 2019-06-11 15:54 UTC (permalink / raw
  To: davem, Pablo Neira Ayuso, Florian Fainelli, Jiri Pirko,
	Jakub Kicinski
  Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
	thomas.petazzoni, Michał Mirosław

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,
 		vlan_priority:3;
 	__be16	vlan_tpid;
 };
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d08b1e19ce9c..43df34c1ebe1 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -3020,6 +3020,11 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
 			match->mask.vlan.vlan_id =
 				ntohs(ext_m_spec->vlan_tci) & 0x0fff;
 
+			match->key.vlan.vlan_cfi =
+				!!(ntohs(ext_h_spec->vlan_tci) & 0x1000);
+			match->mask.vlan.vlan_cfi =
+				!!(ntohs(ext_m_spec->vlan_tci) & 0x1000);
+
 			match->key.vlan.vlan_priority =
 				(ntohs(ext_h_spec->vlan_tci) & 0xe000) >> 13;
 			match->mask.vlan.vlan_priority =
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: ethtool: Allow matching on vlan CFI bit
  2019-06-11 15:54 [PATCH net] net: ethtool: Allow matching on vlan CFI bit Maxime Chevallier
@ 2019-06-11 18:27 ` Jakub Kicinski
  2019-06-12  6:31 ` Toshiaki Makita
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2019-06-11 18:27 UTC (permalink / raw
  To: Maxime Chevallier
  Cc: davem, Pablo Neira Ayuso, Florian Fainelli, Jiri Pirko, netdev,
	linux-kernel, Antoine Tenart, thomas.petazzoni,
	Michał Mirosław

On Tue, 11 Jun 2019 17:54:56 +0200, 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,
>  		vlan_priority:3;
>  	__be16	vlan_tpid;
>  };
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index d08b1e19ce9c..43df34c1ebe1 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -3020,6 +3020,11 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
>  			match->mask.vlan.vlan_id =
>  				ntohs(ext_m_spec->vlan_tci) & 0x0fff;
>  
> +			match->key.vlan.vlan_cfi =
> +				!!(ntohs(ext_h_spec->vlan_tci) & 0x1000);
> +			match->mask.vlan.vlan_cfi =
> +				!!(ntohs(ext_m_spec->vlan_tci) & 0x1000);

nit: since you're only using the output as a boolean, you can apply the
     byteswap to the constant and have it performed at build time.
     Another option is be16_get_bits() from linux/bitfield.h.

>  			match->key.vlan.vlan_priority =
>  				(ntohs(ext_h_spec->vlan_tci) & 0xe000) >> 13;
>  			match->mask.vlan.vlan_priority =

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: ethtool: Allow matching on vlan CFI bit
  2019-06-11 15:54 [PATCH net] net: ethtool: Allow matching on vlan CFI bit Maxime Chevallier
  2019-06-11 18:27 ` Jakub Kicinski
@ 2019-06-12  6:31 ` Toshiaki Makita
  1 sibling, 0 replies; 3+ messages in thread
From: Toshiaki Makita @ 2019-06-12  6:31 UTC (permalink / raw
  To: Maxime Chevallier, davem, Pablo Neira Ayuso, Florian Fainelli,
	Jiri Pirko, Jakub Kicinski
  Cc: netdev, linux-kernel, Antoine Tenart, thomas.petazzoni,
	Michał Mirosław

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-12  6:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-11 15:54 [PATCH net] net: ethtool: Allow matching on vlan CFI bit Maxime Chevallier
2019-06-11 18:27 ` Jakub Kicinski
2019-06-12  6:31 ` Toshiaki Makita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.