netdev - Re: [PATCH net-next v4 09/10] tsnep: Add XDP RX 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: <CAKgT0UcEMDZXUeEVJdVTDOFRvG9uhkVZe252psyASiNN8XqqvQ@mail.gmail.com>
Date:   Tue, 10 Jan 2023 14:30:10 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Gerhard Engleder <gerhard@...leder-embedded.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        edumazet@...gle.com, pabeni@...hat.com
Subject: Re: [PATCH net-next v4 09/10] tsnep: Add XDP RX support

On Tue, Jan 10, 2023 at 1:28 PM Gerhard Engleder
<gerhard@...leder-embedded.com> wrote:
>
> On 10.01.23 18:40, Alexander H Duyck wrote:
> > On Mon, 2023-01-09 at 20:15 +0100, Gerhard Engleder wrote:
> >> If BPF program is set up, then run BPF program for every received frame
> >> and execute the selected action.
> >>
> >> Signed-off-by: Gerhard Engleder <gerhard@...leder-embedded.com>
> >> ---
> >>   drivers/net/ethernet/engleder/tsnep_main.c | 122 ++++++++++++++++++++-
> >>   1 file changed, 120 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
> >> index 451ad1849b9d..002c879639db 100644
> >> --- a/drivers/net/ethernet/engleder/tsnep_main.c
> >> +++ b/drivers/net/ethernet/engleder/tsnep_main.c
> >> @@ -27,6 +27,7 @@
> >>   #include <linux/phy.h>
> >>   #include <linux/iopoll.h>
> >>   #include <linux/bpf.h>
> >> +#include <linux/bpf_trace.h>
> >>
> >>   #define TSNEP_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)
> >>   #define TSNEP_HEADROOM ALIGN(max(TSNEP_SKB_PAD, XDP_PACKET_HEADROOM), 4)
> >> @@ -44,6 +45,9 @@
> >>   #define TSNEP_COALESCE_USECS_MAX     ((ECM_INT_DELAY_MASK >> ECM_INT_DELAY_SHIFT) * \
> >>                                    ECM_INT_DELAY_BASE_US + ECM_INT_DELAY_BASE_US - 1)
> >>
> >> +#define TSNEP_XDP_TX                BIT(0)
> >> +#define TSNEP_XDP_REDIRECT  BIT(1)
> >> +
> >>   enum {
> >>      __TSNEP_DOWN,
> >>   };
> >> @@ -625,6 +629,28 @@ static void tsnep_xdp_xmit_flush(struct tsnep_tx *tx)
> >>      iowrite32(TSNEP_CONTROL_TX_ENABLE, tx->addr + TSNEP_CONTROL);
> >>   }
> >>
> >> +static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter,
> >> +                            struct xdp_buff *xdp,
> >> +                            struct netdev_queue *tx_nq, struct tsnep_tx *tx)
> >> +{
> >> +    struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
> >> +    bool xmit;
> >> +
> >> +    if (unlikely(!xdpf))
> >> +            return false;
> >> +
> >> +    __netif_tx_lock(tx_nq, smp_processor_id());
> >> +
> >> +    /* Avoid transmit queue timeout since we share it with the slow path */
> >> +    txq_trans_cond_update(tx_nq);
> >> +
> >> +    xmit = tsnep_xdp_xmit_frame_ring(xdpf, tx, TSNEP_TX_TYPE_XDP_TX);
> >> +
> >
> > Again the trans_cond_update should be after the xmit and only if it is
> > not indicating it completed the transmit.
>
> tsnep_xdp_xmit_frame_ring() only adds xpdf to the descriptor ring, so it
> cannot complete the transmit. Therefore and in line with your previous
> comment trans_cond_update() should be called here if xpdf is
> successfully placed in the descriptor ring. Is that right?

Yes, that is what I meant by "complete the transmit" is if it places
the xdpf on the descriptor ring then you can update this. Basically
the idea is we should be updating the timer any time a frame goes onto
the ring. It shouldn't be an unconditional update as a stalled ring
could then go undetected.