* Re: [PATCH v7 net-next] virtio-net: support XDP when not more queues
[not found] <1615257107.0376601-1-xuanzhuo@linux.alibaba.com>
@ 2021-03-09 2:47 ` Jason Wang
0 siblings, 0 replies; 4+ messages in thread
From: Jason Wang @ 2021-03-09 2:47 UTC (permalink / raw)
To: Xuan Zhuo
Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, virtualization, bpf, netdev
On 2021/3/9 10:31 上午, Xuan Zhuo wrote:
> On Tue, 9 Mar 2021 10:17:16 +0800, Jason Wang <jasowang@redhat.com> wrote:
>> On 2021/3/8 4:52 下午, Xuan Zhuo wrote:
>>> The number of queues implemented by many virtio backends is limited,
>>> especially some machines have a large number of CPUs. In this case, it
>>> is often impossible to allocate a separate queue for
>>> XDP_TX/XDP_REDIRECT, then xdp cannot be loaded to work, even xdp does
>>> not use the XDP_TX/XDP_REDIRECT.
>>>
>>> This patch allows XDP_TX/XDP_REDIRECT to run by reuse the existing SQ
>>> with __netif_tx_lock() hold when there are not enough queues.
>>>
>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>> Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
>>> ---
>>> v7: 1. use macros to implement get/put
>>> 2. remove 'flag'. (suggested by Jason Wang)
>>>
>>> v6: 1. use __netif_tx_acquire()/__netif_tx_release(). (suggested by Jason Wang)
>>> 2. add note for why not lock. (suggested by Jason Wang)
>>> 3. Use variable 'flag' to record with or without locked. It is not safe to
>>> use curr_queue_pairs in "virtnet_put_xdp_sq", because it may changed after
>>> "virtnet_get_xdp_sq".
>>>
>>> v5: change subject from 'support XDP_TX when not more queues'
>>>
>>> v4: make sparse happy
>>> suggested by Jakub Kicinski
>>>
>>> v3: add warning when no more queues
>>> suggested by Jesper Dangaard Brouer
>>>
>>> drivers/net/virtio_net.c | 55 ++++++++++++++++++++++++++++++++++++------------
>>> 1 file changed, 42 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index ba8e637..5ce40ec 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -195,6 +195,9 @@ struct virtnet_info {
>>> /* # of XDP queue pairs currently used by the driver */
>>> u16 xdp_queue_pairs;
>>>
>>> + /* xdp_queue_pairs may be 0, when xdp is already loaded. So add this. */
>>> + bool xdp_enabled;
>>> +
>>> /* I like... big packets and I cannot lie! */
>>> bool big_packets;
>>>
>>> @@ -481,12 +484,34 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>>> return 0;
>>> }
>>>
>>> -static struct send_queue *virtnet_xdp_sq(struct virtnet_info *vi)
>>> -{
>>> - unsigned int qp;
>>> -
>>> - qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
>>> - return &vi->sq[qp];
>>> +/* when vi->curr_queue_pairs > nr_cpu_ids, the txq/sq is only used for xdp tx on
>>> + * the current cpu, so it does not need to be locked.
>>> + */
>>> +#define virtnet_xdp_get_sq(vi) ({ \
>>> + struct netdev_queue *txq; \
>>> + typeof(vi) v = (vi); \
>>
>> Any reason for not using vi directly?
>>
>> Other looks good.
>>
>> Thanks
> Using vi directly, I got an error by checkpatch.pl:
>
> Macro argument reuse 'vi' - possible side-effects
>
> more info: https://gcc.gnu.org/onlinedocs/cpp/Duplication-of-Side-Effects.html
>
> Thanks.
Ok.
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
>
>>
>>> + unsigned int qp; \
>>> + if (v->curr_queue_pairs > nr_cpu_ids) { \
>>> + qp = v->curr_queue_pairs - v->xdp_queue_pairs; \
>>> + qp += smp_processor_id(); \
>>> + txq = netdev_get_tx_queue(v->dev, qp); \
>>> + __netif_tx_acquire(txq); \
>>> + } else { \
>>> + qp = smp_processor_id() % v->curr_queue_pairs; \
>>> + txq = netdev_get_tx_queue(v->dev, qp); \
>>> + __netif_tx_lock(txq, raw_smp_processor_id()); \
>>> + } \
>>> + v->sq + qp; \
>>> +})
>>> +
>>> +#define virtnet_xdp_put_sq(vi, q) { \
>>> + struct netdev_queue *txq; \
>>> + typeof(vi) v = (vi); \
>>> + txq = netdev_get_tx_queue(v->dev, (q) - v->sq); \
>>> + if (v->curr_queue_pairs > nr_cpu_ids) \
>>> + __netif_tx_release(txq); \
>>> + else \
>>> + __netif_tx_unlock(txq); \
>>> }
>>>
>>> static int virtnet_xdp_xmit(struct net_device *dev,
>>> @@ -512,7 +537,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>>> if (!xdp_prog)
>>> return -ENXIO;
>>>
>>> - sq = virtnet_xdp_sq(vi);
>>> + sq = virtnet_xdp_get_sq(vi);
>>>
>>> if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
>>> ret = -EINVAL;
>>> @@ -560,12 +585,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>>> sq->stats.kicks += kicks;
>>> u64_stats_update_end(&sq->stats.syncp);
>>>
>>> + virtnet_xdp_put_sq(vi, sq);
>>> return ret;
>>> }
>>>
>>> static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
>>> {
>>> - return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
>>> + return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
>>> }
>>>
>>> /* We copy the packet for XDP in the following cases:
>>> @@ -1457,12 +1483,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>> xdp_do_flush();
>>>
>>> if (xdp_xmit & VIRTIO_XDP_TX) {
>>> - sq = virtnet_xdp_sq(vi);
>>> + sq = virtnet_xdp_get_sq(vi);
>>> if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
>>> u64_stats_update_begin(&sq->stats.syncp);
>>> sq->stats.kicks++;
>>> u64_stats_update_end(&sq->stats.syncp);
>>> }
>>> + virtnet_xdp_put_sq(vi, sq);
>>> }
>>>
>>> return received;
>>> @@ -2417,10 +2444,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>>>
>>> /* XDP requires extra queues for XDP_TX */
>>> if (curr_qp + xdp_qp > vi->max_queue_pairs) {
>>> - NL_SET_ERR_MSG_MOD(extack, "Too few free TX rings available");
>>> - netdev_warn(dev, "request %i queues but max is %i\n",
>>> + netdev_warn(dev, "XDP request %i queues but max is %i. XDP_TX and XDP_REDIRECT will operate in a slower locked tx mode.\n",
>>> curr_qp + xdp_qp, vi->max_queue_pairs);
>>> - return -ENOMEM;
>>> + xdp_qp = 0;
>>> }
>>>
>>> old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
>>> @@ -2454,11 +2480,14 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>>> vi->xdp_queue_pairs = xdp_qp;
>>>
>>> if (prog) {
>>> + vi->xdp_enabled = true;
>>> for (i = 0; i < vi->max_queue_pairs; i++) {
>>> rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
>>> if (i == 0 && !old_prog)
>>> virtnet_clear_guest_offloads(vi);
>>> }
>>> + } else {
>>> + vi->xdp_enabled = false;
>>> }
>>>
>>> for (i = 0; i < vi->max_queue_pairs; i++) {
>>> @@ -2526,7 +2555,7 @@ static int virtnet_set_features(struct net_device *dev,
>>> int err;
>>>
>>> if ((dev->features ^ features) & NETIF_F_LRO) {
>>> - if (vi->xdp_queue_pairs)
>>> + if (vi->xdp_enabled)
>>> return -EBUSY;
>>>
>>> if (features & NETIF_F_LRO)
>>> --
>>> 1.8.3.1
>>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v7 net-next] virtio-net: support XDP when not more queues
@ 2021-03-08 8:52 Xuan Zhuo
2021-03-09 2:17 ` Jason Wang
2021-03-09 16:44 ` Michael S. Tsirkin
0 siblings, 2 replies; 4+ messages in thread
From: Xuan Zhuo @ 2021-03-08 8:52 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, virtualization, bpf
The number of queues implemented by many virtio backends is limited,
especially some machines have a large number of CPUs. In this case, it
is often impossible to allocate a separate queue for
XDP_TX/XDP_REDIRECT, then xdp cannot be loaded to work, even xdp does
not use the XDP_TX/XDP_REDIRECT.
This patch allows XDP_TX/XDP_REDIRECT to run by reuse the existing SQ
with __netif_tx_lock() hold when there are not enough queues.
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
v7: 1. use macros to implement get/put
2. remove 'flag'. (suggested by Jason Wang)
v6: 1. use __netif_tx_acquire()/__netif_tx_release(). (suggested by Jason Wang)
2. add note for why not lock. (suggested by Jason Wang)
3. Use variable 'flag' to record with or without locked. It is not safe to
use curr_queue_pairs in "virtnet_put_xdp_sq", because it may changed after
"virtnet_get_xdp_sq".
v5: change subject from 'support XDP_TX when not more queues'
v4: make sparse happy
suggested by Jakub Kicinski
v3: add warning when no more queues
suggested by Jesper Dangaard Brouer
drivers/net/virtio_net.c | 55 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 13 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba8e637..5ce40ec 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -195,6 +195,9 @@ struct virtnet_info {
/* # of XDP queue pairs currently used by the driver */
u16 xdp_queue_pairs;
+ /* xdp_queue_pairs may be 0, when xdp is already loaded. So add this. */
+ bool xdp_enabled;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
@@ -481,12 +484,34 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
return 0;
}
-static struct send_queue *virtnet_xdp_sq(struct virtnet_info *vi)
-{
- unsigned int qp;
-
- qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
- return &vi->sq[qp];
+/* when vi->curr_queue_pairs > nr_cpu_ids, the txq/sq is only used for xdp tx on
+ * the current cpu, so it does not need to be locked.
+ */
+#define virtnet_xdp_get_sq(vi) ({ \
+ struct netdev_queue *txq; \
+ typeof(vi) v = (vi); \
+ unsigned int qp; \
+ if (v->curr_queue_pairs > nr_cpu_ids) { \
+ qp = v->curr_queue_pairs - v->xdp_queue_pairs; \
+ qp += smp_processor_id(); \
+ txq = netdev_get_tx_queue(v->dev, qp); \
+ __netif_tx_acquire(txq); \
+ } else { \
+ qp = smp_processor_id() % v->curr_queue_pairs; \
+ txq = netdev_get_tx_queue(v->dev, qp); \
+ __netif_tx_lock(txq, raw_smp_processor_id()); \
+ } \
+ v->sq + qp; \
+})
+
+#define virtnet_xdp_put_sq(vi, q) { \
+ struct netdev_queue *txq; \
+ typeof(vi) v = (vi); \
+ txq = netdev_get_tx_queue(v->dev, (q) - v->sq); \
+ if (v->curr_queue_pairs > nr_cpu_ids) \
+ __netif_tx_release(txq); \
+ else \
+ __netif_tx_unlock(txq); \
}
static int virtnet_xdp_xmit(struct net_device *dev,
@@ -512,7 +537,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
if (!xdp_prog)
return -ENXIO;
- sq = virtnet_xdp_sq(vi);
+ sq = virtnet_xdp_get_sq(vi);
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
ret = -EINVAL;
@@ -560,12 +585,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
sq->stats.kicks += kicks;
u64_stats_update_end(&sq->stats.syncp);
+ virtnet_xdp_put_sq(vi, sq);
return ret;
}
static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
{
- return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
+ return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
}
/* We copy the packet for XDP in the following cases:
@@ -1457,12 +1483,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
xdp_do_flush();
if (xdp_xmit & VIRTIO_XDP_TX) {
- sq = virtnet_xdp_sq(vi);
+ sq = virtnet_xdp_get_sq(vi);
if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
u64_stats_update_begin(&sq->stats.syncp);
sq->stats.kicks++;
u64_stats_update_end(&sq->stats.syncp);
}
+ virtnet_xdp_put_sq(vi, sq);
}
return received;
@@ -2417,10 +2444,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
/* XDP requires extra queues for XDP_TX */
if (curr_qp + xdp_qp > vi->max_queue_pairs) {
- NL_SET_ERR_MSG_MOD(extack, "Too few free TX rings available");
- netdev_warn(dev, "request %i queues but max is %i\n",
+ netdev_warn(dev, "XDP request %i queues but max is %i. XDP_TX and XDP_REDIRECT will operate in a slower locked tx mode.\n",
curr_qp + xdp_qp, vi->max_queue_pairs);
- return -ENOMEM;
+ xdp_qp = 0;
}
old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
@@ -2454,11 +2480,14 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
vi->xdp_queue_pairs = xdp_qp;
if (prog) {
+ vi->xdp_enabled = true;
for (i = 0; i < vi->max_queue_pairs; i++) {
rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
if (i == 0 && !old_prog)
virtnet_clear_guest_offloads(vi);
}
+ } else {
+ vi->xdp_enabled = false;
}
for (i = 0; i < vi->max_queue_pairs; i++) {
@@ -2526,7 +2555,7 @@ static int virtnet_set_features(struct net_device *dev,
int err;
if ((dev->features ^ features) & NETIF_F_LRO) {
- if (vi->xdp_queue_pairs)
+ if (vi->xdp_enabled)
return -EBUSY;
if (features & NETIF_F_LRO)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v7 net-next] virtio-net: support XDP when not more queues
2021-03-08 8:52 Xuan Zhuo
@ 2021-03-09 2:17 ` Jason Wang
2021-03-09 16:44 ` Michael S. Tsirkin
1 sibling, 0 replies; 4+ messages in thread
From: Jason Wang @ 2021-03-09 2:17 UTC (permalink / raw)
To: Xuan Zhuo, netdev
Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, virtualization, bpf
On 2021/3/8 4:52 下午, Xuan Zhuo wrote:
> The number of queues implemented by many virtio backends is limited,
> especially some machines have a large number of CPUs. In this case, it
> is often impossible to allocate a separate queue for
> XDP_TX/XDP_REDIRECT, then xdp cannot be loaded to work, even xdp does
> not use the XDP_TX/XDP_REDIRECT.
>
> This patch allows XDP_TX/XDP_REDIRECT to run by reuse the existing SQ
> with __netif_tx_lock() hold when there are not enough queues.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
> ---
> v7: 1. use macros to implement get/put
> 2. remove 'flag'. (suggested by Jason Wang)
>
> v6: 1. use __netif_tx_acquire()/__netif_tx_release(). (suggested by Jason Wang)
> 2. add note for why not lock. (suggested by Jason Wang)
> 3. Use variable 'flag' to record with or without locked. It is not safe to
> use curr_queue_pairs in "virtnet_put_xdp_sq", because it may changed after
> "virtnet_get_xdp_sq".
>
> v5: change subject from 'support XDP_TX when not more queues'
>
> v4: make sparse happy
> suggested by Jakub Kicinski
>
> v3: add warning when no more queues
> suggested by Jesper Dangaard Brouer
>
> drivers/net/virtio_net.c | 55 ++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index ba8e637..5ce40ec 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -195,6 +195,9 @@ struct virtnet_info {
> /* # of XDP queue pairs currently used by the driver */
> u16 xdp_queue_pairs;
>
> + /* xdp_queue_pairs may be 0, when xdp is already loaded. So add this. */
> + bool xdp_enabled;
> +
> /* I like... big packets and I cannot lie! */
> bool big_packets;
>
> @@ -481,12 +484,34 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
> return 0;
> }
>
> -static struct send_queue *virtnet_xdp_sq(struct virtnet_info *vi)
> -{
> - unsigned int qp;
> -
> - qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
> - return &vi->sq[qp];
> +/* when vi->curr_queue_pairs > nr_cpu_ids, the txq/sq is only used for xdp tx on
> + * the current cpu, so it does not need to be locked.
> + */
> +#define virtnet_xdp_get_sq(vi) ({ \
> + struct netdev_queue *txq; \
> + typeof(vi) v = (vi); \
Any reason for not using vi directly?
Other looks good.
Thanks
> + unsigned int qp; \
> + if (v->curr_queue_pairs > nr_cpu_ids) { \
> + qp = v->curr_queue_pairs - v->xdp_queue_pairs; \
> + qp += smp_processor_id(); \
> + txq = netdev_get_tx_queue(v->dev, qp); \
> + __netif_tx_acquire(txq); \
> + } else { \
> + qp = smp_processor_id() % v->curr_queue_pairs; \
> + txq = netdev_get_tx_queue(v->dev, qp); \
> + __netif_tx_lock(txq, raw_smp_processor_id()); \
> + } \
> + v->sq + qp; \
> +})
> +
> +#define virtnet_xdp_put_sq(vi, q) { \
> + struct netdev_queue *txq; \
> + typeof(vi) v = (vi); \
> + txq = netdev_get_tx_queue(v->dev, (q) - v->sq); \
> + if (v->curr_queue_pairs > nr_cpu_ids) \
> + __netif_tx_release(txq); \
> + else \
> + __netif_tx_unlock(txq); \
> }
>
> static int virtnet_xdp_xmit(struct net_device *dev,
> @@ -512,7 +537,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> if (!xdp_prog)
> return -ENXIO;
>
> - sq = virtnet_xdp_sq(vi);
> + sq = virtnet_xdp_get_sq(vi);
>
> if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
> ret = -EINVAL;
> @@ -560,12 +585,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> sq->stats.kicks += kicks;
> u64_stats_update_end(&sq->stats.syncp);
>
> + virtnet_xdp_put_sq(vi, sq);
> return ret;
> }
>
> static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> {
> - return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
> + return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> }
>
> /* We copy the packet for XDP in the following cases:
> @@ -1457,12 +1483,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> xdp_do_flush();
>
> if (xdp_xmit & VIRTIO_XDP_TX) {
> - sq = virtnet_xdp_sq(vi);
> + sq = virtnet_xdp_get_sq(vi);
> if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
> u64_stats_update_begin(&sq->stats.syncp);
> sq->stats.kicks++;
> u64_stats_update_end(&sq->stats.syncp);
> }
> + virtnet_xdp_put_sq(vi, sq);
> }
>
> return received;
> @@ -2417,10 +2444,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>
> /* XDP requires extra queues for XDP_TX */
> if (curr_qp + xdp_qp > vi->max_queue_pairs) {
> - NL_SET_ERR_MSG_MOD(extack, "Too few free TX rings available");
> - netdev_warn(dev, "request %i queues but max is %i\n",
> + netdev_warn(dev, "XDP request %i queues but max is %i. XDP_TX and XDP_REDIRECT will operate in a slower locked tx mode.\n",
> curr_qp + xdp_qp, vi->max_queue_pairs);
> - return -ENOMEM;
> + xdp_qp = 0;
> }
>
> old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
> @@ -2454,11 +2480,14 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> vi->xdp_queue_pairs = xdp_qp;
>
> if (prog) {
> + vi->xdp_enabled = true;
> for (i = 0; i < vi->max_queue_pairs; i++) {
> rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> if (i == 0 && !old_prog)
> virtnet_clear_guest_offloads(vi);
> }
> + } else {
> + vi->xdp_enabled = false;
> }
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> @@ -2526,7 +2555,7 @@ static int virtnet_set_features(struct net_device *dev,
> int err;
>
> if ((dev->features ^ features) & NETIF_F_LRO) {
> - if (vi->xdp_queue_pairs)
> + if (vi->xdp_enabled)
> return -EBUSY;
>
> if (features & NETIF_F_LRO)
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v7 net-next] virtio-net: support XDP when not more queues
2021-03-08 8:52 Xuan Zhuo
2021-03-09 2:17 ` Jason Wang
@ 2021-03-09 16:44 ` Michael S. Tsirkin
1 sibling, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2021-03-09 16:44 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jason Wang, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, virtualization, bpf
On Mon, Mar 08, 2021 at 04:52:16PM +0800, Xuan Zhuo wrote:
> The number of queues implemented by many virtio backends is limited,
> especially some machines have a large number of CPUs. In this case, it
> is often impossible to allocate a separate queue for
> XDP_TX/XDP_REDIRECT, then xdp cannot be loaded to work, even xdp does
> not use the XDP_TX/XDP_REDIRECT.
>
> This patch allows XDP_TX/XDP_REDIRECT to run by reuse the existing SQ
> with __netif_tx_lock() hold when there are not enough queues.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
> ---
> v7: 1. use macros to implement get/put
> 2. remove 'flag'. (suggested by Jason Wang)
>
> v6: 1. use __netif_tx_acquire()/__netif_tx_release(). (suggested by Jason Wang)
> 2. add note for why not lock. (suggested by Jason Wang)
> 3. Use variable 'flag' to record with or without locked. It is not safe to
> use curr_queue_pairs in "virtnet_put_xdp_sq", because it may changed after
> "virtnet_get_xdp_sq".
>
> v5: change subject from 'support XDP_TX when not more queues'
>
> v4: make sparse happy
> suggested by Jakub Kicinski
>
> v3: add warning when no more queues
> suggested by Jesper Dangaard Brouer
>
> drivers/net/virtio_net.c | 55 ++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index ba8e637..5ce40ec 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -195,6 +195,9 @@ struct virtnet_info {
> /* # of XDP queue pairs currently used by the driver */
> u16 xdp_queue_pairs;
>
> + /* xdp_queue_pairs may be 0, when xdp is already loaded. So add this. */
> + bool xdp_enabled;
> +
> /* I like... big packets and I cannot lie! */
> bool big_packets;
>
> @@ -481,12 +484,34 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
> return 0;
> }
>
> -static struct send_queue *virtnet_xdp_sq(struct virtnet_info *vi)
> -{
> - unsigned int qp;
> -
> - qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
> - return &vi->sq[qp];
> +/* when vi->curr_queue_pairs > nr_cpu_ids, the txq/sq is only used for xdp tx on
> + * the current cpu, so it does not need to be locked.
> + */
pls also explain why these are macros not inline functions in the
comment.
> +#define virtnet_xdp_get_sq(vi) ({ \
> + struct netdev_queue *txq; \
> + typeof(vi) v = (vi); \
> + unsigned int qp; \
empty line here after variable definitions.
same elsewhere
> + if (v->curr_queue_pairs > nr_cpu_ids) { \
> + qp = v->curr_queue_pairs - v->xdp_queue_pairs; \
> + qp += smp_processor_id(); \
> + txq = netdev_get_tx_queue(v->dev, qp); \
> + __netif_tx_acquire(txq); \
> + } else { \
> + qp = smp_processor_id() % v->curr_queue_pairs; \
> + txq = netdev_get_tx_queue(v->dev, qp); \
> + __netif_tx_lock(txq, raw_smp_processor_id()); \
> + } \
> + v->sq + qp; \
> +})
> +
> +#define virtnet_xdp_put_sq(vi, q) { \
> + struct netdev_queue *txq; \
> + typeof(vi) v = (vi); \
> + txq = netdev_get_tx_queue(v->dev, (q) - v->sq); \
> + if (v->curr_queue_pairs > nr_cpu_ids) \
> + __netif_tx_release(txq); \
> + else \
> + __netif_tx_unlock(txq); \
> }
> static int virtnet_xdp_xmit(struct net_device *dev,
> @@ -512,7 +537,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> if (!xdp_prog)
> return -ENXIO;
>
> - sq = virtnet_xdp_sq(vi);
> + sq = virtnet_xdp_get_sq(vi);
>
> if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
> ret = -EINVAL;
> @@ -560,12 +585,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> sq->stats.kicks += kicks;
> u64_stats_update_end(&sq->stats.syncp);
>
> + virtnet_xdp_put_sq(vi, sq);
> return ret;
> }
>
> static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> {
> - return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
> + return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> }
>
> /* We copy the packet for XDP in the following cases:
> @@ -1457,12 +1483,13 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> xdp_do_flush();
>
> if (xdp_xmit & VIRTIO_XDP_TX) {
> - sq = virtnet_xdp_sq(vi);
> + sq = virtnet_xdp_get_sq(vi);
> if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
> u64_stats_update_begin(&sq->stats.syncp);
> sq->stats.kicks++;
> u64_stats_update_end(&sq->stats.syncp);
> }
> + virtnet_xdp_put_sq(vi, sq);
> }
>
> return received;
> @@ -2417,10 +2444,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>
> /* XDP requires extra queues for XDP_TX */
> if (curr_qp + xdp_qp > vi->max_queue_pairs) {
> - NL_SET_ERR_MSG_MOD(extack, "Too few free TX rings available");
> - netdev_warn(dev, "request %i queues but max is %i\n",
> + netdev_warn(dev, "XDP request %i queues but max is %i. XDP_TX and XDP_REDIRECT will operate in a slower locked tx mode.\n",
> curr_qp + xdp_qp, vi->max_queue_pairs);
> - return -ENOMEM;
> + xdp_qp = 0;
> }
>
> old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
> @@ -2454,11 +2480,14 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> vi->xdp_queue_pairs = xdp_qp;
>
> if (prog) {
> + vi->xdp_enabled = true;
> for (i = 0; i < vi->max_queue_pairs; i++) {
> rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> if (i == 0 && !old_prog)
> virtnet_clear_guest_offloads(vi);
> }
> + } else {
> + vi->xdp_enabled = false;
> }
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> @@ -2526,7 +2555,7 @@ static int virtnet_set_features(struct net_device *dev,
> int err;
>
> if ((dev->features ^ features) & NETIF_F_LRO) {
> - if (vi->xdp_queue_pairs)
> + if (vi->xdp_enabled)
> return -EBUSY;
>
> if (features & NETIF_F_LRO)
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-09 16:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1615257107.0376601-1-xuanzhuo@linux.alibaba.com>
2021-03-09 2:47 ` [PATCH v7 net-next] virtio-net: support XDP when not more queues Jason Wang
2021-03-08 8:52 Xuan Zhuo
2021-03-09 2:17 ` Jason Wang
2021-03-09 16:44 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).