| Message ID | 20180423135619.7179-15-bjorn.topel@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Introducing AF_XDP support | expand |
On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel <bjorn.topel@gmail.com> wrote: > From: Magnus Karlsson <magnus.karlsson@intel.com> > > In this commit, a new getsockopt is added: XDP_STATISTICS. This is > used to obtain stats from the sockets. > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> > +static int xsk_getsockopt(struct socket *sock, int level, int optname, > + char __user *optval, int __user *optlen) > +{ > + struct sock *sk = sock->sk; > + struct xdp_sock *xs = xdp_sk(sk); > + int len; > + > + if (level != SOL_XDP) > + return -ENOPROTOOPT; > + > + if (get_user(len, optlen)) > + return -EFAULT; > + if (len < 0) > + return -EINVAL; > + > + switch (optname) { > + case XDP_STATISTICS: > + { > + struct xdp_statistics stats; > + > + if (len != sizeof(stats)) > + return -EINVAL; > + > + mutex_lock(&xs->mutex); > + stats.rx_dropped = xs->rx_dropped; > + stats.rx_invalid_descs = xskq_nb_invalid_descs(xs->rx); > + stats.tx_invalid_descs = xskq_nb_invalid_descs(xs->tx); > + mutex_unlock(&xs->mutex); > + > + if (copy_to_user(optval, &stats, sizeof(stats))) > + return -EFAULT; > + return 0; For forward compatibility, could allow caller to pass a struct larger than stats and return the number of bytes filled in. The lock can also be elided with something like gnet_stats, but it is probably taken rarely enough that that is not worth the effort, at least right now.
On Tue, Apr 24, 2018 at 6:58 PM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel <bjorn.topel@gmail.com> wrote: >> From: Magnus Karlsson <magnus.karlsson@intel.com> >> >> In this commit, a new getsockopt is added: XDP_STATISTICS. This is >> used to obtain stats from the sockets. >> >> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> > >> +static int xsk_getsockopt(struct socket *sock, int level, int optname, >> + char __user *optval, int __user *optlen) >> +{ >> + struct sock *sk = sock->sk; >> + struct xdp_sock *xs = xdp_sk(sk); >> + int len; >> + >> + if (level != SOL_XDP) >> + return -ENOPROTOOPT; >> + >> + if (get_user(len, optlen)) >> + return -EFAULT; >> + if (len < 0) >> + return -EINVAL; >> + >> + switch (optname) { >> + case XDP_STATISTICS: >> + { >> + struct xdp_statistics stats; >> + >> + if (len != sizeof(stats)) >> + return -EINVAL; >> + >> + mutex_lock(&xs->mutex); >> + stats.rx_dropped = xs->rx_dropped; >> + stats.rx_invalid_descs = xskq_nb_invalid_descs(xs->rx); >> + stats.tx_invalid_descs = xskq_nb_invalid_descs(xs->tx); >> + mutex_unlock(&xs->mutex); >> + >> + if (copy_to_user(optval, &stats, sizeof(stats))) >> + return -EFAULT; >> + return 0; > > For forward compatibility, could allow caller to pass a struct larger > than stats and return the number of bytes filled in. Yes definitely. Will fix right away. > The lock can also be elided with something like gnet_stats, but it is probably > taken rarely enough that that is not worth the effort, at least right now. Will put this on the ever expanding todo list for future patches ;-). Thanks: Magnus
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h index e2ea878d025c..77b88c4efe98 100644 --- a/include/uapi/linux/if_xdp.h +++ b/include/uapi/linux/if_xdp.h @@ -38,6 +38,7 @@ struct sockaddr_xdp { #define XDP_UMEM_REG 3 #define XDP_UMEM_FILL_RING 4 #define XDP_UMEM_COMPLETION_RING 5 +#define XDP_STATISTICS 6 struct xdp_umem_reg { __u64 addr; /* Start of packet data area */ @@ -46,6 +47,12 @@ struct xdp_umem_reg { __u32 frame_headroom; /* Frame head room */ }; +struct xdp_statistics { + __u64 rx_dropped; /* Dropped for reasons other than invalid desc */ + __u64 rx_invalid_descs; /* Dropped due to invalid descriptor */ + __u64 tx_invalid_descs; /* Dropped due to invalid descriptor */ +}; + /* Pgoff for mmaping the rings */ #define XDP_PGOFF_RX_RING 0 #define XDP_PGOFF_TX_RING 0x80000000 diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index 1c0b1ea10453..6d115609f9ed 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -520,6 +520,46 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname, return -ENOPROTOOPT; } +static int xsk_getsockopt(struct socket *sock, int level, int optname, + char __user *optval, int __user *optlen) +{ + struct sock *sk = sock->sk; + struct xdp_sock *xs = xdp_sk(sk); + int len; + + if (level != SOL_XDP) + return -ENOPROTOOPT; + + if (get_user(len, optlen)) + return -EFAULT; + if (len < 0) + return -EINVAL; + + switch (optname) { + case XDP_STATISTICS: + { + struct xdp_statistics stats; + + if (len != sizeof(stats)) + return -EINVAL; + + mutex_lock(&xs->mutex); + stats.rx_dropped = xs->rx_dropped; + stats.rx_invalid_descs = xskq_nb_invalid_descs(xs->rx); + stats.tx_invalid_descs = xskq_nb_invalid_descs(xs->tx); + mutex_unlock(&xs->mutex); + + if (copy_to_user(optval, &stats, sizeof(stats))) + return -EFAULT; + return 0; + } + default: + break; + } + + return -EOPNOTSUPP; +} + static int xsk_mmap(struct file *file, struct socket *sock, struct vm_area_struct *vma) { @@ -575,7 +615,7 @@ static const struct proto_ops xsk_proto_ops = { .listen = sock_no_listen, .shutdown = sock_no_shutdown, .setsockopt = xsk_setsockopt, - .getsockopt = sock_no_getsockopt, + .getsockopt = xsk_getsockopt, .sendmsg = xsk_sendmsg, .recvmsg = sock_no_recvmsg, .mmap = xsk_mmap, diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h index ea3be9f9e95a..7686ef355b83 100644 --- a/net/xdp/xsk_queue.h +++ b/net/xdp/xsk_queue.h @@ -36,6 +36,11 @@ struct xsk_queue { /* Common functions operating for both RXTX and umem queues */ +static inline u64 xskq_nb_invalid_descs(struct xsk_queue *q) +{ + return q ? q->invalid_descs : 0; +} + static inline u32 xskq_nb_avail(struct xsk_queue *q, u32 dcnt) { u32 entries = q->prod_tail - q->cons_tail;