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  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:	Fri, 31 Oct 2008 17:16:33 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Stephen Hemminger <shemminger@...tta.com>
cc:	David Miller <davem@...emloft.net>, dada1@...mosbay.com,
	zbr@...emap.net, ilpo.jarvinen@...sinki.fi, rjw@...k.pl,
	mingo@...e.hu, s0mbre@...rvice.net.ru, a.p.zijlstra@...llo.nl,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	efault@....de, akpm@...ux-foundation.org
Subject: Re: [tbench regression fixes]: digging out smelly deadmen.

Stephen Hemminger <shemminger@...tta.com> wrote:

>On Fri, 31 Oct 2008 16:51:44 -0700 (PDT)
>David Miller <davem@...emloft.net> wrote:
[...]
>> However, I do like Stephen's suggestion that maybe we can get rid of
>> this ->last_rx thing by encapsulating the logic completely in the
>> bonding driver.
>
>Since bonding driver doesn't actually see the rx packets, that isn't
>really possible.  But it would be possible to change last_rx from a variable
>to an function pointer, so that device's could apply other logic to derive
>the last value.  One example would be to keep it per cpu and then take the
>maximum.

	I suspect it could also be tucked away in skb_bond_should_drop,
which is called both by the standard input path and the VLAN accelerated
path to see if the packet should be tossed (e.g., it arrived on an
inactive bonding slave).

	Since last_rx is part of struct net_device, I don't think any
additional bonding internals knowledge would be needed.  It could be
arranged to only update last_rx for devices that are actually bonding
slaves.

	Just off the top of my head (haven't tested this), something
like this:

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c8bcb59..ed1e58f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1743,22 +1743,24 @@ static inline int skb_bond_should_drop(struct sk_buff *skb)
 	struct net_device *dev = skb->dev;
 	struct net_device *master = dev->master;
 
-	if (master &&
-	    (dev->priv_flags & IFF_SLAVE_INACTIVE)) {
-		if ((dev->priv_flags & IFF_SLAVE_NEEDARP) &&
-		    skb->protocol == __constant_htons(ETH_P_ARP))
-			return 0;
-
-		if (master->priv_flags & IFF_MASTER_ALB) {
-			if (skb->pkt_type != PACKET_BROADCAST &&
-			    skb->pkt_type != PACKET_MULTICAST)
+	if (master) {
+		dev->last_rx = jiffies;
+		if (dev->priv_flags & IFF_SLAVE_INACTIVE)) {
+			if ((dev->priv_flags & IFF_SLAVE_NEEDARP) &&
+			    skb->protocol == __constant_htons(ETH_P_ARP))
 				return 0;
-		}
-		if (master->priv_flags & IFF_MASTER_8023AD &&
-		    skb->protocol == __constant_htons(ETH_P_SLOW))
-			return 0;
 
-		return 1;
+			if (master->priv_flags & IFF_MASTER_ALB) {
+				if (skb->pkt_type != PACKET_BROADCAST &&
+				    skb->pkt_type != PACKET_MULTICAST)
+					return 0;
+			}
+			if (master->priv_flags & IFF_MASTER_8023AD &&
+			    skb->protocol == __constant_htons(ETH_P_SLOW))
+				return 0;
+
+			return 1;
+		}
 	}
 	return 0;
 }


	That doesn't move the storage out of struct net_device, but it
does stop the updates for devices that aren't bonding slaves.  It could
probably be refined further to only update when the ARP monitor is
running (the gizmo that uses last_rx).

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists