All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, davem@davemloft.net,
	anthony.l.nguyen@intel.com, kuba@kernel.org, bjorn@kernel.org,
	magnus.karlsson@intel.com,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Subject: Re: [PATCH intel-next 2/2] ice: introduce XDP Tx fallback path
Date: Tue, 01 Jun 2021 14:38:03 +0200	[thread overview]
Message-ID: <87czt5dal0.fsf@toke.dk> (raw)
In-Reply-To: <20210601113236.42651-3-maciej.fijalkowski@intel.com>

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> Under rare circumstances there might be a situation where a requirement
> of having a XDP Tx queue per core could not be fulfilled and some of the
> Tx resources would have to be shared between cores. This yields a need
> for placing accesses to xdp_rings array onto critical section protected
> by spinlock.
>
> Design of handling such scenario is to at first find out how many queues
> are there that XDP could use. Any number that is not less than the half
> of a count of cores of platform is allowed. XDP queue count < cpu count
> is signalled via new VSI state ICE_VSI_XDP_FALLBACK which carries the
> information further down to Rx rings where new ICE_TX_XDP_LOCKED is set
> based on the mentioned VSI state. This ring flag indicates that locking
> variants for getting/putting xdp_ring need to be used in fast path.
>
> For XDP_REDIRECT the impact on standard case (one XDP ring per CPU) can
> be reduced a bit by providing a separate ndo_xdp_xmit and swap it at
> configuration time. However, due to the fact that net_device_ops struct
> is a const, it is not possible to replace a single ndo, so for the
> locking variant of ndo_xdp_xmit, whole net_device_ops needs to be
> replayed.
>
> It has an impact on performance (1-2 %) of a non-fallback path as
> branches are introduced.

I generally feel this is the right approach, although the performance
impact is a bit unfortunately, obviously. Maybe it could be avoided by
the use of static_branch? I.e., keep a global refcount of how many
netdevs are using the locked path and only activate the check in the
fast path while that refcount is >0?

-Toke


WARNING: multiple messages have this Message-ID (diff)
From: Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH intel-next 2/2] ice: introduce XDP Tx fallback path
Date: Tue, 01 Jun 2021 14:38:03 +0200	[thread overview]
Message-ID: <87czt5dal0.fsf@toke.dk> (raw)
In-Reply-To: <20210601113236.42651-3-maciej.fijalkowski@intel.com>

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> Under rare circumstances there might be a situation where a requirement
> of having a XDP Tx queue per core could not be fulfilled and some of the
> Tx resources would have to be shared between cores. This yields a need
> for placing accesses to xdp_rings array onto critical section protected
> by spinlock.
>
> Design of handling such scenario is to at first find out how many queues
> are there that XDP could use. Any number that is not less than the half
> of a count of cores of platform is allowed. XDP queue count < cpu count
> is signalled via new VSI state ICE_VSI_XDP_FALLBACK which carries the
> information further down to Rx rings where new ICE_TX_XDP_LOCKED is set
> based on the mentioned VSI state. This ring flag indicates that locking
> variants for getting/putting xdp_ring need to be used in fast path.
>
> For XDP_REDIRECT the impact on standard case (one XDP ring per CPU) can
> be reduced a bit by providing a separate ndo_xdp_xmit and swap it at
> configuration time. However, due to the fact that net_device_ops struct
> is a const, it is not possible to replace a single ndo, so for the
> locking variant of ndo_xdp_xmit, whole net_device_ops needs to be
> replayed.
>
> It has an impact on performance (1-2 %) of a non-fallback path as
> branches are introduced.

I generally feel this is the right approach, although the performance
impact is a bit unfortunately, obviously. Maybe it could be avoided by
the use of static_branch? I.e., keep a global refcount of how many
netdevs are using the locked path and only activate the check in the
fast path while that refcount is >0?

-Toke


  reply	other threads:[~2021-06-01 12:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 11:32 [PATCH intel-next 0/2] ice: bring up XDP_TX back to life Maciej Fijalkowski
2021-06-01 11:32 ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-01 11:32 ` [PATCH intel-next 1/2] ice: unify xdp_rings accesses Maciej Fijalkowski
2021-06-01 11:32   ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-01 11:32 ` [PATCH intel-next 2/2] ice: introduce XDP Tx fallback path Maciej Fijalkowski
2021-06-01 11:32   ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-01 12:38   ` Toke Høiland-Jørgensen [this message]
2021-06-01 12:38     ` Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-06-08 12:12     ` Maciej Fijalkowski
2021-06-08 12:12       ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-08 21:43       ` Toke Høiland-Jørgensen
2021-06-08 21:43         ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-06-03  0:52   ` Nguyen, Anthony L
2021-06-03  0:52     ` [Intel-wired-lan] " Nguyen, Anthony L
2021-06-08 12:13     ` Maciej Fijalkowski
2021-06-08 12:13       ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-03  8:42   ` Jesper Dangaard Brouer
2021-06-03  8:42     ` [Intel-wired-lan] " Jesper Dangaard Brouer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czt5dal0.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.