Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
From: Jeff Garzik [EMAIL PROTECTED] Date: Fri, 10 Aug 2007 16:56:17 -0400 All this is currently checked into the 'eflags' branch of git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git But when everybody is happy with it, IMO we should get it into net-2.6.24.git, as it enables LRO. I've backed out the ETHTOOL LRO stuff from Auke, and applied your patches to net-2.6.24 We can get rid of that NETIF_F_LRO thing, since as you observed it is indeed superfluous. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
Rick Jones wrote: David Miller wrote: From: Ben Greear [EMAIL PROTECTED] Date: Fri, 10 Aug 2007 15:40:02 -0700 For GSO on output, is there a generic fallback for any driver that does not specifically implement GSO? Absolutely, in fact that's mainly what it's there for. I don't think there is any issue. The knob is there via ethtool for people who really want to disable it. Just to be paranoid (who me?) we are then at a point where what happened a couple months ago with forwarding between 10G and IPoIB won't happen again - where things failed because a 10G NIC had LRO enabled by default? we still have the NETIF_F_LRO flag which Jeff will keep around. Perhaps the IPoIB code can force this to _off_ when setting it up? (or at least warn about it). Auke - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
All this is currently checked into the 'eflags' branch of git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git But when everybody is happy with it, IMO we should get it into net-2.6.24.git, as it enables LRO. Jeff - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
Jeff Garzik wrote: This patch copies Auke in adding NETIF_F_LRO. Is that just for temporary merging, or does the net core really not touch it at all? Because, logically, if NETIF_F_LRO exists nowhere else but this patch, we should not add it to dev-features. LRO knowledge can be contained entirely within the driver, if the net core never tests NETIF_F_LRO. I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be applied to any other NETIF_F_XXX flag: if the net stack isn't using it, it's a piece of information specific to that driver. I believe LRO is going to have to be disabled for routing/bridging, so the stack will probably need to become aware of it at some point... Thanks, Ben -- Ben Greear [EMAIL PROTECTED] Candela Technologies Inc http://www.candelatech.com - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
Jeff Garzik wrote:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4a616d7..559a4dc 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -341,6 +341,7 @@ struct net_device
#define NETIF_F_GSO2048/* Enable software GSO. */
#define NETIF_F_LLTX 4096/* LockLess TX */
#define NETIF_F_MULTI_QUEUE16384 /* Has multiple TX/RX queues */
+#define NETIF_F_LRO32768 /* large receive offload */
/* Segmentation offload features */
#define NETIF_F_GSO_SHIFT 16
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 2ab0a60..6e8563e 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -109,6 +109,32 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data)
return 0;
}
+/* the following list of flags are the same as their associated
+ * NETIF_F_xxx values in include/linux/netdevice.h
+ */
+static const u32 flags_dup_features =
+ ETH_FLAG_LRO;
+
+u32 ethtool_op_get_flags(struct net_device *dev)
+{
+ /* in the future, this function will probably contain additional
+* handling for flags which are not so easily handled
+* by a simple masking operation
+*/
+
+ return dev-features flags_dup_features;
+}
+
+int ethtool_op_set_flags(struct net_device *dev, u32 data)
+{
+ if (data ETH_FLAG_LRO)
+ dev-features |= NETIF_F_LRO;
+ else
+ dev-features = ~NETIF_F_LRO;
+
And, a side discussion:
This patch copies Auke in adding NETIF_F_LRO. Is that just for
temporary merging, or does the net core really not touch it at all?
Because, logically, if NETIF_F_LRO exists nowhere else but this patch,
we should not add it to dev-features. LRO knowledge can be contained
entirely within the driver, if the net core never tests NETIF_F_LRO.
I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be
applied to any other NETIF_F_XXX flag: if the net stack isn't using it,
it's a piece of information specific to that driver.
Jeff
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
From: Ben Greear [EMAIL PROTECTED] Date: Fri, 10 Aug 2007 14:11:24 -0700 Jeff Garzik wrote: This patch copies Auke in adding NETIF_F_LRO. Is that just for temporary merging, or does the net core really not touch it at all? Because, logically, if NETIF_F_LRO exists nowhere else but this patch, we should not add it to dev-features. LRO knowledge can be contained entirely within the driver, if the net core never tests NETIF_F_LRO. I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be applied to any other NETIF_F_XXX flag: if the net stack isn't using it, it's a piece of information specific to that driver. I believe LRO is going to have to be disabled for routing/bridging, so the stack will probably need to become aware of it at some point... The packet will be GSO'd on output I believe, so it won't break anything. Alternatively, we could make the driver only LRO accumulate if the packet is unicast and matches one of the MAC's programmed into the chip. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
David Miller wrote: From: Ben Greear [EMAIL PROTECTED] I believe LRO is going to have to be disabled for routing/bridging, so the stack will probably need to become aware of it at some point... The packet will be GSO'd on output I believe, so it won't break anything. Alternatively, we could make the driver only LRO accumulate if the packet is unicast and matches one of the MAC's programmed into the chip. I think even this would fail if you are doing something clever with NAT or other iptables stuff. Probably we're going to have to put this in the hands of the users..who hopefully can determine whether they can allow LRO or not... For GSO on output, is there a generic fallback for any driver that does not specifically implement GSO? Thanks, Ben -- Ben Greear [EMAIL PROTECTED] Candela Technologies Inc http://www.candelatech.com - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
From: Ben Greear [EMAIL PROTECTED] Date: Fri, 10 Aug 2007 15:40:02 -0700 For GSO on output, is there a generic fallback for any driver that does not specifically implement GSO? Absolutely, in fact that's mainly what it's there for. I don't think there is any issue. The knob is there via ethtool for people who really want to disable it. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] Add ETHTOOL_[GS]FLAGS sub-ioctls
David Miller wrote: From: Ben Greear [EMAIL PROTECTED] Date: Fri, 10 Aug 2007 15:40:02 -0700 For GSO on output, is there a generic fallback for any driver that does not specifically implement GSO? Absolutely, in fact that's mainly what it's there for. I don't think there is any issue. The knob is there via ethtool for people who really want to disable it. Just to be paranoid (who me?) we are then at a point where what happened a couple months ago with forwarding between 10G and IPoIB won't happen again - where things failed because a 10G NIC had LRO enabled by default? rick jones - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
