> 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. Hardware GRO guarantees that packets can be
> re-segmented by TSO/GSO to reconstruct the original packet stream.
>
> Cc: Ariel Elior <ariel.el...@cavium.com>
> Cc: everest-linux...@cavium.com
> Signed-off-by: Michael Chan <michael.c...@broadcom.com>
> ---
> Documentation/networking/netdev-features.txt | 7 +++++++
> include/linux/netdev_features.h | 5 ++++-
> net/core/dev.c | 13 +++++++++++++
> net/core/ethtool.c | 1 +
> 4 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/networking/netdev-features.txt
> b/Documentation/networking/netdev-features.txt
> index 7413eb0..d76d332 100644
> --- a/Documentation/networking/netdev-features.txt
> +++ b/Documentation/networking/netdev-features.txt
> @@ -163,3 +163,10 @@ 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 packet stream.
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index dc8b489..d18ef6f 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -77,6 +77,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
> @@ -96,6 +98,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)
> @@ -193,7 +196,7 @@ enum {
> * If upper/master device has these features disabled, they must be disabled
> * on all lower/slave devices as well.
> */
> -#define NETIF_F_UPPER_DISABLES NETIF_F_LRO
> +#define NETIF_F_UPPER_DISABLES (NETIF_F_LRO | NETIF_F_GRO_HW)
>
> /* changeable features with no special hardware requirements */
> #define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 30b5fe3..09c2ad0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7392,6 +7392,19 @@ static netdev_features_t
> netdev_fix_features(struct net_device *dev,
> features &= ~dev->gso_partial_features;
> }
>
> + if (features & NETIF_F_GRO_HW) {
> + /* Hardware GRO depends on GRO. */
> + if (!(features & NETIF_F_GRO)) {
While at it, perhaps also make it dependent on NETIF_F_RXCSUM?
> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since
> no GRO feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
> + /* Hardware GRO and LRO are mutually exclusive. */
> + if (features & NETIF_F_LRO) {
> + netdev_dbg(dev, "Dropping NETIF_F_LRO since
> GRO_HW is set.\n");
> + features &= ~NETIF_F_LRO;
Isn't this considered to be breaking an existing API?
After this, while NETIF_F_GRO_HW is published an application trying to
set NETIF_F_LRO and then query its state would discover it failed
[while previously it could have succeeded, such as for bnx2]
While I understand the need to make sure core doesn't enable
two competing aggregation offloads, why make GRO_HW > LRO?
I understand it's probably the better one, but until LRO gets deprecated
isn't it safer to do this limitation the opposite way?
I.e., make sure NETIF_F_GRO_HW can't be set as long as NETIF_F_LRO is set?
> + }
> + }
> +
> 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",
> --
> 1.8.3.1
