On Fri, Apr 21, 2023 at 09:02:56PM +0200, Gerhard Engleder wrote:
> On 20.04.23 22:17, Maciej Fijalkowski wrote:
> > On Tue, Apr 18, 2023 at 09:04:59PM +0200, Gerhard Engleder wrote:
> > > Send and complete XSK pool frames within TX NAPI context. NAPI context
> > > is triggered by ndo_xsk_wakeup.
> > >
> > > Test results with A53 1.2GHz:
> > >
> > > xdpsock txonly copy mode:
> > > pps pkts 1.00
> > > tx 284,409 11,398,144
> > > Two CPUs with 100% and 10% utilization.
> > >
> > > xdpsock txonly zero-copy mode:
> > > pps pkts 1.00
> > > tx 511,929 5,890,368
> > > Two CPUs with 100% and 1% utilization.
> >
> > Hmm, I think l2fwd ZC numbers should be included here not in the previous
> > patch?
>
> Will be done.
>
> > >
> > > Packet rate increases and CPU utilization is reduced.
> > >
> > > Signed-off-by: Gerhard Engleder <gerhard@xxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/net/ethernet/engleder/tsnep.h | 2 +
> > > drivers/net/ethernet/engleder/tsnep_main.c | 127 +++++++++++++++++++--
> > > 2 files changed, 119 insertions(+), 10 deletions(-)
> > >
>
> (...)
>
> > > static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
> > > {
> > > struct tsnep_tx_entry *entry;
> > > struct netdev_queue *nq;
> > > + int xsk_frames = 0;
> > > int budget = 128;
> > > int length;
> > > int count;
> > > @@ -676,7 +771,7 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
> > > if ((entry->type & TSNEP_TX_TYPE_SKB) &&
> > > skb_shinfo(entry->skb)->nr_frags > 0)
> > > count += skb_shinfo(entry->skb)->nr_frags;
> > > - else if (!(entry->type & TSNEP_TX_TYPE_SKB) &&
> > > + else if ((entry->type & TSNEP_TX_TYPE_XDP) &&
> > > xdp_frame_has_frags(entry->xdpf))
> > > count += xdp_get_shared_info_from_frame(entry->xdpf)->nr_frags;
> > > @@ -705,9 +800,11 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
> > > if (entry->type & TSNEP_TX_TYPE_SKB)
> > > napi_consume_skb(entry->skb, napi_budget);
> > > - else
> > > + else if (entry->type & TSNEP_TX_TYPE_XDP)
> > > xdp_return_frame_rx_napi(entry->xdpf);
> > > - /* xdpf is union with skb */
> > > + else
> > > + xsk_frames++;
> > > + /* xdpf and zc are union with skb */
> > > entry->skb = NULL;
> > > tx->read = (tx->read + count) & TSNEP_RING_MASK;
> > > @@ -718,6 +815,14 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
> > > budget--;
> > > } while (likely(budget));
> > > + if (tx->xsk_pool) {
> > > + if (xsk_frames)
> > > + xsk_tx_completed(tx->xsk_pool, xsk_frames);
> > > + if (xsk_uses_need_wakeup(tx->xsk_pool))
> > > + xsk_set_tx_need_wakeup(tx->xsk_pool);
> > > + tsnep_xdp_xmit_zc(tx);
> >
> > would be good to signal to NAPI if we are done with the work or is there a
> > need to be rescheduled (when you didn't manage to consume all of the descs
> > from XSK Tx ring).
>
> In my opinion this is already done. If some budget is left, then we are
> done and tsnep_tx_poll() returns true to signal work is complete. If
> buget gets zero, then tsnep_tx_poll() returns false to signal work is
> not complete. This return value is considered for the NAPI signaling
> by tsnep_poll().
i was only referring to tsnep_xdp_xmit_zc() being void on return. Thing is
that there is currently no way you would tell to napi there is more work
to be done. you should do so if desc_available == batch. That would mean
there might be still descriptors on XSK Tx ring ready to be consumed. NAPI
budget is out of the picture here.
