netdev - Re: bond interface arp, vlan and trunk / network question

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]

Message-ID: <20090423192254.GA1618@psychotron.englab.brq.redhat.com>
Date:	Thu, 23 Apr 2009 21:22:55 +0200
From:	Jiri Pirko <jpirko@...hat.com>
To:	Jay Vosburgh <fubar@...ibm.com>
Cc:	stefan novak <lms.brubaker@...il.com>,
	Eric Dumazet <dada1@...mosbay.com>,
	linux-kernel@...r.kernel.org,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: bond interface arp, vlan and trunk / network question

Thu, Apr 23, 2009 at 08:34:20PM CEST, fubar@...ibm.com wrote:
>Jiri Pirko <jpirko@...hat.com> wrote:
>>Thu, Apr 23, 2009 at 04:59:41PM CEST, fubar@...ibm.com wrote:
>>>Jiri Pirko <jpirko@...hat.com> wrote:
>[...]
>>>>>+{
>>>>>+	struct net_device *dev = skb->dev;
>>>>>+	struct net_device *master = dev->master;
>>>>>+
>>>>>+	if (master)
>>>>>+		return bond_handle_frame_hook(skb);
>>>>
>>>>Maybe this hook can be called from netif_receive_skb() directly. You would safe
>>>>at least 2 dereferences, 1 if check. You would also need to add
>>>>"skb->dev->master &&" to if in __vlan_hwaccel_rx() and vlan_gro_common().
>>>
>>>	This won't work, because the VLAN code reassigns skb->dev to the
>>>VLAN device before calling netif_receive_skb.
>>
>>Sure, but bond_should_drop is called before it actually reassigns that. So the
>>check in bond_should_drop tests "original_dev->master".
>>
>>I had on mind something like following:
>>
>>Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>>
>>diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>>index c67fe6f..87a7334 100644
>>--- a/net/8021q/vlan_core.c
>>+++ b/net/8021q/vlan_core.c
>>@@ -11,7 +11,7 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
>> 	if (netpoll_rx(skb))
>> 		return NET_RX_DROP;
>>
>>-	if (skb_bond_should_drop(skb))
>>+	if (skb->dev->master && bond_handle_frame_hook(skb))
>[...]
>
>	Yah, ok, I see what you mean.  The same could be accomplished by
>turning skb_bond_should_drop back into an inline in the header file, and

Well no exactly. In vlan_x certainly yes but in netif_receive_skb() you safe
something by not checking ->master twice and 2 dereferences. Just noting when I
see the waste...

>hiding the fiddly bits from the calling context.
>
>	It's pretty grotty no matter how it's done; I'd prefer to avoid
>the whole hook business, but I haven't thought of a less bad way.

Yes I agree hooking this sucks :/ Anyway, is there any movement to eliminate
all these hooks from netif_receive_skb() ?

	Jirka
>
>	-J
>
>---
>	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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