On 11/16/20, Xuan Zhuo <xuanz...@linux.alibaba.com> wrote:
> On Mon, 16 Nov 2020 12:13:21 +0300, Denis Kirjanov <k...@linux-powerpc.org>
> wrote:
>> On 11/16/20, Xuan Zhuo <xuanz...@linux.alibaba.com> wrote:
>> > When we write all cq items to tx, we have to wait for a new event based
>> > on poll to indicate that it is writable. But the current writability is
>> > triggered based on whether tx is full or not, and In fact, when tx is
>> > dissatisfied, the user of cq's item may not necessarily get it, because
>> > it
>> > may still be occupied by the network card. In this case, we need to
>> > know
>> > when cq is available, so this patch adds a socket option, When the user
>> > configures this option using setsockopt, when cq is available, a
>> > readable event is generated for all xsk bound to this umem.
>> >
>> > I can't find a better description of this event,
>> > I think it can also be 'readable', although it is indeed different from
>> > the 'readable' of the new data. But the overhead of xsk checking
>> > whether
>> > cq or rx is readable is small.
>> >
>> > Signed-off-by: Xuan Zhuo <xuanz...@linux.alibaba.com>
>> > ---
>> > include/net/xdp_sock.h | 1 +
>> > include/uapi/linux/if_xdp.h | 1 +
>> > net/xdp/xsk.c | 28 ++++++++++++++++++++++++++++
>> > 3 files changed, 30 insertions(+)
>> >
>> > diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
>> > index 1a9559c..faf5b1a 100644
>> > --- a/include/net/xdp_sock.h
>> > +++ b/include/net/xdp_sock.h
>> > @@ -49,6 +49,7 @@ struct xdp_sock {
>> > struct xsk_buff_pool *pool;
>> > u16 queue_id;
>> > bool zc;
>> > + bool cq_event;
>> > enum {
>> > XSK_READY = 0,
>> > XSK_BOUND,
>> > diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
>> > index a78a809..2dba3cb 100644
>> > --- a/include/uapi/linux/if_xdp.h
>> > +++ b/include/uapi/linux/if_xdp.h
>> > @@ -63,6 +63,7 @@ struct xdp_mmap_offsets {
>> > #define XDP_UMEM_COMPLETION_RING 6
>> > #define XDP_STATISTICS 7
>> > #define XDP_OPTIONS 8
>> > +#define XDP_CQ_EVENT 9
>> >
>> > struct xdp_umem_reg {
>> > __u64 addr; /* Start of packet data area */
>> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
>> > index cfbec39..0c53403 100644
>> > --- a/net/xdp/xsk.c
>> > +++ b/net/xdp/xsk.c
>> > @@ -285,7 +285,16 @@ void __xsk_map_flush(void)
>> >
>> > void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries)
>> > {
>> > + struct xdp_sock *xs;
>> > +
>> > xskq_prod_submit_n(pool->cq, nb_entries);
>> > +
>> > + rcu_read_lock();
>> > + list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
>> > + if (xs->cq_event)
>> > + sock_def_readable(&xs->sk);
>> > + }
>> > + rcu_read_unlock();
>> > }
>> > EXPORT_SYMBOL(xsk_tx_completed);
>> >
>> > @@ -495,6 +504,9 @@ static __poll_t xsk_poll(struct file *file, struct
>> > socket *sock,
>> > __xsk_sendmsg(sk);
>> > }
>> >
>> > + if (xs->cq_event && pool->cq && !xskq_prod_is_empty(pool->cq))
>> > + mask |= EPOLLIN | EPOLLRDNORM;
>> > +
>> > if (xs->rx && !xskq_prod_is_empty(xs->rx))
>> > mask |= EPOLLIN | EPOLLRDNORM;
>> > if (xs->tx && !xskq_cons_is_full(xs->tx))
>> > @@ -882,6 +894,22 @@ static int xsk_setsockopt(struct socket *sock, int
>> > level, int optname,
>> > mutex_unlock(&xs->mutex);
>> > return err;
>> > }
>> > + case XDP_CQ_EVENT:
>> > + {
>> > + int cq_event;
>> > +
>> > + if (optlen < sizeof(cq_event))
>> > + return -EINVAL;
>> > + if (copy_from_sockptr(&cq_event, optval, sizeof(cq_event)))
>> > + return -EFAULT;
>> > +
>> > + if (cq_event)
>> > + xs->cq_event = true;
>> > + else
>> > + xs->cq_event = false;
>>
>> It's false by default, isn't it?
>
> I add cq_event inside "xdp_sock", that is got by sk_alloc, this call
> sk_prot_alloc by __GFP_ZERO. So I think it is false.
Right, I meant that what's the point to set it explicitly to 'false'?
>
> Thanks.
>
>>
>> > +
>> > + return 0;
>> > + }
>> > default:
>> > break;
>> > }
>> > --
>> > 1.8.3.1
>> >
>> >
>
