[PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule

[PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule

[Maxime Chevallier]

When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
which could contain matches based on the ethernet header, such as the
MAC address, the VLAN tag or the ethertype.

Only the ethtype field is specific to the ether flow, the MAC and vlan
fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 net/core/ethtool.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4d1011b2e24f..01ceba556341 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
 	match->mask.basic.n_proto = htons(0xffff);
 
 	switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
+	case ETHER_FLOW: {
+		const struct ethhdr *ether_spec, *ether_m_spec;
+
+		ether_spec = &fs->h_u.ether_spec;
+		ether_m_spec = &fs->m_u.ether_spec;
+
+		if (ether_m_spec->h_proto) {
+			match->key.basic.n_proto = ether_spec->h_proto;
+			match->mask.basic.n_proto = ether_m_spec->h_proto;
+		}
+		}
+		break;
 	case TCP_V4_FLOW:
 	case UDP_V4_FLOW: {
 		const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
-- 
2.20.1

Re: [PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule

[Pablo Neira Ayuso]

On Wed, Jun 26, 2019 at 10:44:03AM +0200, Maxime Chevallier wrote:
> When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
> which could contain matches based on the ethernet header, such as the
> MAC address, the VLAN tag or the ethertype.
> 
> Only the ethtype field is specific to the ether flow, the MAC and vlan
> fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  net/core/ethtool.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 4d1011b2e24f..01ceba556341 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
>  	match->mask.basic.n_proto = htons(0xffff);
>  
>  	switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
> +	case ETHER_FLOW: {
> +		const struct ethhdr *ether_spec, *ether_m_spec;
> +
> +		ether_spec = &fs->h_u.ether_spec;
> +		ether_m_spec = &fs->m_u.ether_spec;
> +
> +		if (ether_m_spec->h_proto) {
> +			match->key.basic.n_proto = ether_spec->h_proto;
> +			match->mask.basic.n_proto = ether_m_spec->h_proto;
> +		}

I see some drivers in the tree also interpret the h_source and h_dest
fields?

https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/sfc/falcon/ethtool.c#L1182

Probably good to address this in this patch too?

Thanks.

Re: [PATCH net-next] net: ethtool: Allow parsing ETHER_FLOW types when using flow_rule

[Maxime Chevallier]

Hi Pablo,

On Wed, 26 Jun 2019 10:58:46 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

>On Wed, Jun 26, 2019 at 10:44:03AM +0200, Maxime Chevallier wrote:
>> When parsing an ethtool_rx_flow_spec, users can specify an ethernet flow
>> which could contain matches based on the ethernet header, such as the
>> MAC address, the VLAN tag or the ethertype.
>> 
>> Only the ethtype field is specific to the ether flow, the MAC and vlan
>> fields are processed using the special FLOW_EXT and FLOW_MAC_EXT flags.
>> 
>> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>> ---
>>  net/core/ethtool.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>> 
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 4d1011b2e24f..01ceba556341 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -2883,6 +2883,18 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
>>  	match->mask.basic.n_proto = htons(0xffff);
>>  
>>  	switch (fs->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS)) {
>> +	case ETHER_FLOW: {
>> +		const struct ethhdr *ether_spec, *ether_m_spec;
>> +
>> +		ether_spec = &fs->h_u.ether_spec;
>> +		ether_m_spec = &fs->m_u.ether_spec;
>> +
>> +		if (ether_m_spec->h_proto) {
>> +			match->key.basic.n_proto = ether_spec->h_proto;
>> +			match->mask.basic.n_proto = ether_m_spec->h_proto;
>> +		}  
>
>I see some drivers in the tree also interpret the h_source and h_dest
>fields?

Ah yes you're right. I assumed these fields were specific to the
FLOW_MAC_EXT flags, but by looking into the ethtool code, it seems we
do need to handle the h_source and h_dest fields.

I'll respin with these fields added.

Thanks for the review,

Maxime