| 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
| ||
|
Date: Tue, 12 Dec 2017 07:57:11 -0800
From: Alexander Duyck <alexander.duyck@...il.com>
To: Michael Chan <michael.chan@...adcom.com>
Cc: David Miller <davem@...emloft.net>,
Netdev <netdev@...r.kernel.org>,
Andrew Gospodarek <andrew.gospodarek@...adcom.com>,
Ariel Elior <Ariel.Elior@...ium.com>,
everest-linux-l2@...ium.com
Subject: Re: [PATCH net-next v4 1/5] net: Introduce NETIF_F_GRO_HW.
On Mon, Dec 11, 2017 at 3:41 AM, Michael Chan <michael.chan@...adcom.com> wrote:
> 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 <Ariel.Elior@...ium.com>
> Cc: everest-linux-l2@...ium.com
> Signed-off-by: Michael Chan <michael.chan@...adcom.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.
> 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.
> + 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",
Powered by blists - more mailing lists