Re: [PATCH net-next v4 1/5] net: Introduce NETIF_F_GRO_HW.
flat view
From: Alexander Duyck <hidden>
Date: 2017-12-12 15:57:13
On Mon, Dec 11, 2017 at 3:41 AM, Michael Chan [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware GRO. With this flag, we can now independently turn on or off hardware GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to control hardware GRO and so it cannot be independently turned on or off without affecting GRO. Hardware GRO (just like GRO) guarantees that packets can be re-segmented by TSO/GSO to reconstruct the original packet stream. Logically, GRO_HW should depend on GRO since it a subset, but we will let individual drivers enforce this dependency as they see fit. Since NETIF_F_GRO is not propagated between upper and lower devices, NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other words, a lower device can independent have GRO/GRO_HW enabled or disabled and no feature propagation is required. This will preserve the current GRO behavior. This can be changed later if we decide to propagate GRO/ GRO_HW/RXCSUM from upper to lower devices. Cc: Ariel Elior <redacted> Cc: <redacted> Signed-off-by: Michael Chan <michael.chan@broadcom.com> --- Documentation/networking/netdev-features.txt | 8 ++++++++ include/linux/netdev_features.h | 3 +++ net/core/dev.c | 10 ++++++++++ net/core/ethtool.c | 1 + 4 files changed, 22 insertions(+)diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt index 7413eb0..8f36527 100644 --- a/Documentation/networking/netdev-features.txt +++ b/Documentation/networking/netdev-features.txt@@ -163,3 +163,11 @@ This requests that the NIC receive all possible frames, including errored frames (such as bad FCS, etc). This can be helpful when sniffing a link with bad packets on it. Some NICs may receive more packets if also put into normal PROMISC mode. + +* rx-gro-hw + +This requests that the NIC enables Hardware GRO (generic receive offload). +Hardware GRO is basically the exact reverse of TSO, and is generally +stricter than Hardware LRO. A packet stream merged by Hardware GRO must +be re-segmentable by GSO or TSO back to the exact original packet stream. +Hardware GRO is dependent on GRO and RXCSUM.
This last line should probably be updated since we made the change that dropped the dependency on software GRO.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h index b1b0ca7..db84c51 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h@@ -78,6 +78,8 @@ enum { NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */ NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */ + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */ + /* * Add your fresh new feature above and remember to update * netdev_features_strings[] in net/core/ethtool.c and maybe@@ -97,6 +99,7 @@ enum { #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST) #define NETIF_F_FSO __NETIF_F(FSO) #define NETIF_F_GRO __NETIF_F(GRO) +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW) #define NETIF_F_GSO __NETIF_F(GSO) #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST) #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)diff --git a/net/core/dev.c b/net/core/dev.c index 8aa2f70..bf0149e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c@@ -7424,6 +7424,16 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, features &= ~dev->gso_partial_features; } + if (!(features & NETIF_F_RXCSUM)) { + /* NETIF_F_GRO_HW implies doing RXCSUM. If the user does not + * want to enable RXCSUM, logically, we should disable GRO_HW. + */
It might be in our interest here to explain why. Specifically that "The Rx checksum can no longer be verified once a packet is modified as a result of GRO_HW, therefore we cannot allow GRO_HW to be enabled when RXCSUM is disabled." or something to that effect. Otherwise there isn't a good explanation as to why GRO_HW implies this.
quoted hunk ↗ jump to hunk
+ if (features & NETIF_F_GRO_HW) { + netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since no RXCSUM feature.\n"); + features &= ~NETIF_F_GRO_HW; + } + } + return features; }diff --git a/net/core/ethtool.c b/net/core/ethtool.c index f8fcf45..50a7920 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c@@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info) [NETIF_F_LLTX_BIT] = "tx-lockless", [NETIF_F_NETNS_LOCAL_BIT] = "netns-local", [NETIF_F_GRO_BIT] = "rx-gro", + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw", [NETIF_F_LRO_BIT] = "rx-lro", [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",