netdev - Re: [PATCH net-next v2 02/12] net: dsa: add Broadcom tag hook

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] [day] [month] [year] [list]

Date:	Wed, 06 Aug 2014 13:16:23 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
CC:	davem@...emloft.net, linville@...driver.com
Subject: Re: [PATCH net-next v2 02/12] net: dsa: add Broadcom tag hook

On 08/06/2014 11:37 AM, Florian Fainelli wrote:
> On 08/06/2014 07:47 AM, Alexander Duyck wrote:
>> On 08/05/2014 03:31 PM, Florian Fainelli wrote:
>>> Register a fake Ethertype for the Broadcom tag to allow us to hook into
>>> a given Ethernet device receive path and parse this Broadcom tag.
>>>
>>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> 
> [snip]
> 
>>> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
>>> index f405e0592407..6b67653d5283 100644
>>> --- a/net/ethernet/eth.c
>>> +++ b/net/ethernet/eth.c
>>> @@ -186,6 +186,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>>>  
>>>  	if (unlikely(netdev_uses_trailer_tags(dev)))
>>>  		return htons(ETH_P_TRAILER);
>>> +	if (netdev_uses_brcm_tags(dev))
>>> +		return htons(ETH_P_BRCMTAG);
>>>  
>>>  	if (likely(ntohs(eth->h_proto) >= ETH_P_802_3_MIN))
>>>  		return eth->h_proto;
>>>
>>
>> Maybe we should consider some change to this logic.  Maybe a bit flag in
>> the dsa_switch_tree structure that indicates if eth_type_trans should
>> override the protocol or not.
> 
> I just tested something like this, which becomes protocol agnostic:
> 
> if (unlikely(netdev_uses_dsa(dev)))
> 	return dsa_tag_protocol(dev);
> 
> include/linux/netdevice.h:
> static inline bool netdev_uses_dsa(struct net_device *dev)
> {
> #ifdef CONFIG_NET_DSA
> 	return dev->dsa_ptr != NULL;
> #else
> 	return false;
> #endif
> }
> 
> include/net/dsa.h:
> static __be16 netdev_dsa_protocol(struct net_device *dev)
> {
> #ifdef CONFIG_NET_DSA
> 	struct dsa_switch_tree *dst = dev->dsa_ptr;
> 	return dst->tag_protocol;
> #endif
> }
> 
>> Otherwise this is going to start becoming
>> a rather large conditional statement if we have to add a new if every
>> time a new switch is added.
> 
> Agreed.
> 

I think EDSA does not have same logic for inserting the tags.  You may
want to look into that to determine if this is acceptable for that case
or not.

Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html