netdev - Re: [PATCH net-next v3 1/4] net: dsa: add support for DSA rx offloading via metadata dst

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:   Sat, 12 Nov 2022 01:05:10 +0100
From:   Felix Fietkau <nbd@....name>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 1/4] net: dsa: add support for DSA rx
 offloading via metadata dst

On 12.11.22 00:37, Vladimir Oltean wrote:
> On Thu, Nov 10, 2022 at 10:22:08PM +0100, Felix Fietkau wrote:
>> If a metadata dst is present with the type METADATA_HW_PORT_MUX on a dsa cpu
>> port netdev, assume that it carries the port number and that there is no DSA
>> tag present in the skb data.
>> 
>> Signed-off-by: Felix Fietkau <nbd@....name>
>>  				else
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 64b14f655b23..0b67622cf905 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/netdevice.h>
>>  #include <linux/sysfs.h>
>>  #include <linux/ptp_classify.h>
>> +#include <net/dst_metadata.h>
>>  
>>  #include "dsa_priv.h"
>>  
>> @@ -216,6 +217,7 @@ static bool dsa_skb_defer_rx_timestamp(struct dsa_slave_priv *p,
>>  static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>>  			  struct packet_type *pt, struct net_device *unused)
>>  {
>> +	struct metadata_dst *md_dst = skb_metadata_dst(skb);
>>  	struct dsa_port *cpu_dp = dev->dsa_ptr;
>>  	struct sk_buff *nskb = NULL;
>>  	struct dsa_slave_priv *p;
>> @@ -229,7 +231,22 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>>  	if (!skb)
>>  		return 0;
>>  
>> -	nskb = cpu_dp->rcv(skb, dev);
>> +	if (md_dst && md_dst->type == METADATA_HW_PORT_MUX) {
>> +		unsigned int port = md_dst->u.port_info.port_id;
>> +
>> +		skb_dst_set(skb, NULL);
> 
> If you insist on not using the refcounting feature and free your
> metadata_dst in the master's remove() function, that's going to
> invalidate absolutely any point I'm trying to make. Normally I'd leave
> you alone, however I really don't like that this is also forcing DSA to
> not use the refcount, and therefore, that it's forcing any other driver
> to do the same as mtk_eth_soc. Not sure how that's gonna scale in the
> hypothetical future when there will be a DSA master which can offload
> RX DSA tags, *and* the switch can change tagging protocols dynamically
> (which will force the master to allocate/free its metadata dst's at
> runtime too). I guess that will be for me to figure out, which I don't
> like.
> 
> Jakub, what do you think? Refcounting or no refcounting?
If think that refcounting for the metadata dst is useful in this case, I 
can replace the skb_dst_set call with skb_dst_drop() in v4, so it would 
work for both cases.

- Felix