netdev - Re: [PATCH net-next 12/16] gve: DQO: Add core netdev features

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANH7hM5R8CR2pP5oLY-C7OcmdnX1xc9tZuimobPrdXiOGmi6BA@mail.gmail.com>
Date:   Thu, 24 Jun 2021 16:55:08 -0700
From:   Bailey Forrest <bcf@...gle.com>
To:     "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
Cc:     "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Willem de Bruijn <willemb@...gle.com>,
        Catherine Sullivan <csully@...gle.com>
Subject: Re: [PATCH net-next 12/16] gve: DQO: Add core netdev features

On Thu, Jun 24, 2021 at 4:18 PM Samudrala, Sridhar
<sridhar.samudrala@...el.com> wrote:
>
> On 6/24/2021 11:06 AM, Bailey Forrest wrote:
> > Add napi netdev device registration, interrupt handling and initial tx
> > and rx polling stubs. The stubs will be filled in follow-on patches.
> >
> > Also:
> > - LRO feature advertisement and handling
> > - Also update ethtool logic
> >
> > Signed-off-by: Bailey Forrest <bcf@...gle.com>
> > Reviewed-by: Willem de Bruijn <willemb@...gle.com>
> > Reviewed-by: Catherine Sullivan <csully@...gle.com>
> > ---
> >   drivers/net/ethernet/google/gve/Makefile      |   2 +-
> >   drivers/net/ethernet/google/gve/gve.h         |   2 +
> >   drivers/net/ethernet/google/gve/gve_adminq.c  |   2 +
> >   drivers/net/ethernet/google/gve/gve_dqo.h     |  32 +++
> >   drivers/net/ethernet/google/gve/gve_ethtool.c |  12 +-
> >   drivers/net/ethernet/google/gve/gve_main.c    | 188 ++++++++++++++++--
> >   drivers/net/ethernet/google/gve/gve_rx_dqo.c  |  24 +++
> >   drivers/net/ethernet/google/gve/gve_tx_dqo.c  |  23 +++
> >   8 files changed, 260 insertions(+), 25 deletions(-)
> >   create mode 100644 drivers/net/ethernet/google/gve/gve_dqo.h
> >   create mode 100644 drivers/net/ethernet/google/gve/gve_rx_dqo.c
> >   create mode 100644 drivers/net/ethernet/google/gve/gve_tx_dqo.c
> >
> > diff --git a/drivers/net/ethernet/google/gve/Makefile b/drivers/net/ethernet/google/gve/Makefile
> > index 0143f4471e42..b9a6be76531b 100644
> > --- a/drivers/net/ethernet/google/gve/Makefile
> > +++ b/drivers/net/ethernet/google/gve/Makefile
> > @@ -1,4 +1,4 @@
> >   # Makefile for the Google virtual Ethernet (gve) driver
> >
> >   obj-$(CONFIG_GVE) += gve.o
> > -gve-objs := gve_main.o gve_tx.o gve_rx.o gve_ethtool.o gve_adminq.o gve_utils.o
> > +gve-objs := gve_main.o gve_tx.o gve_tx_dqo.o gve_rx.o gve_rx_dqo.o gve_ethtool.o gve_adminq.o gve_utils.o
> > diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> > index 8a2a8d125090..d6bf0466ae8b 100644
> > --- a/drivers/net/ethernet/google/gve/gve.h
> > +++ b/drivers/net/ethernet/google/gve/gve.h
> > @@ -45,6 +45,8 @@
> >   /* PTYPEs are always 10 bits. */
> >   #define GVE_NUM_PTYPES      1024
> >
> > +#define GVE_RX_BUFFER_SIZE_DQO 2048
> > +
> >   /* Each slot in the desc ring has a 1:1 mapping to a slot in the data ring */
> >   struct gve_rx_desc_queue {
> >       struct gve_rx_desc *desc_ring; /* the descriptor ring */
> > diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> > index cf017a499119..5bb56b454541 100644
> > --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> > +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> > @@ -714,6 +714,8 @@ int gve_adminq_describe_device(struct gve_priv *priv)
> >       if (gve_is_gqi(priv)) {
> >               err = gve_set_desc_cnt(priv, descriptor);
> >       } else {
> > +             /* DQO supports LRO. */
> > +             priv->dev->hw_features |= NETIF_F_LRO;
>
> Shouldn't this be NETIF_F_HW_GRO?
> Also, what does DQO stands for?

DQO stands for "Dual Queue Out of order completions"

For now we only support LRO. HW GRO support may come in the future.

The reason for this is I was unable to get HW GRO to function in
conjunction with `napi_gro_frags()`. On our system, we need to use
`napi_gro_frags()` to achieve good performance. I did not see any
other drivers which support NETIF_F_HW_GRO and also use
`napi_gro_frags()`.

>
> <snip>
>