linux-kernel - Re: [PATCH net-next v2 3/4] net: lan966x: Add basic XDP support

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221108112601.605326-1-alexandr.lobakin@intel.com>
Date:   Tue,  8 Nov 2022 12:26:01 +0100
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, ast@...nel.org,
        daniel@...earbox.net, hawk@...nel.org, john.fastabend@...il.com,
        linux@...linux.org.uk
Subject: Re: [PATCH net-next v2 3/4] net: lan966x: Add basic XDP support

From: Horatiu Vultur <horatiu.vultur@...rochip.com>
Date: Mon, 7 Nov 2022 22:26:18 +0100

> The 11/07/2022 17:13, Alexander Lobakin wrote:
> 
> Hi Olek,
> 
> > 
> > From: Alexander Lobakin <alexander.lobakin@...el.com>
> > 
> > From: Horatiu Vultur <horatiu.vultur@...rochip.com>
> > Date: Sun, 6 Nov 2022 22:11:53 +0100
> > 
> > > Introduce basic XDP support to lan966x driver. Currently the driver
> > > supports only the actions XDP_PASS, XDP_DROP and XDP_ABORTED.
> > >
> > > Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> > > ---
> > >  .../net/ethernet/microchip/lan966x/Makefile   |  3 +-
> > >  .../ethernet/microchip/lan966x/lan966x_fdma.c | 11 ++-
> > >  .../ethernet/microchip/lan966x/lan966x_main.c |  5 ++
> > >  .../ethernet/microchip/lan966x/lan966x_main.h | 13 +++
> > >  .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
> > >  5 files changed, 111 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> > 
> > [...]
> > 
> > > +bool lan966x_xdp_port_present(struct lan966x_port *port)
> > > +{
> > > +     return !!port->xdp_prog;
> > > +}
> > 
> > Why uninline such a simple check? I realize you want to keep all XDP
> > stuff inside in the separate file, but doesn't this one looks too
> > much?
> 
> I was kind of hoping that the compiler will inline it for me.
> But I can add it in the header file to inline it.

That is very unlikely for the compilers to uninline an extern
function. LTO is able to do that, but even then it's not
guaranteed. So I'd keep it in a header file as an inline.

> 
> > 
> > > +
> > > +int lan966x_xdp_port_init(struct lan966x_port *port)
> > > +{
> > > +     struct lan966x *lan966x = port->lan966x;
> > > +
> > > +     return xdp_rxq_info_reg(&port->xdp_rxq, port->dev, 0,
> > > +                             lan966x->napi.napi_id);
> > > +}
> > > +
> > > +void lan966x_xdp_port_deinit(struct lan966x_port *port)
> > > +{
> > > +     if (xdp_rxq_info_is_reg(&port->xdp_rxq))
> > > +             xdp_rxq_info_unreg(&port->xdp_rxq);
> > > +}
> > > --
> > > 2.38.0
> > 
> > Thanks,
> > Olek
> 
> -- 
> /Horatiu

Thanks,
Olek