Re: [PATCH bpf-next 14/15] xsk: statistics support
On Tue, Apr 24, 2018 at 6:58 PM, Willem de Bruijn wrote: > On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel wrote: >> From: Magnus Karlsson >> >> In this commit, a new getsockopt is added: XDP_STATISTICS. This is >> used to obtain stats from the sockets. >> >> Signed-off-by: Magnus Karlsson > >> +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
Re: [PATCH bpf-next 14/15] xsk: statistics support
On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel wrote: > From: Magnus Karlsson > > In this commit, a new getsockopt is added: XDP_STATISTICS. This is > used to obtain stats from the sockets. > > Signed-off-by: Magnus Karlsson > +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.
[PATCH bpf-next 14/15] xsk: statistics support
From: Magnus Karlsson In this commit, a new getsockopt is added: XDP_STATISTICS. This is used to obtain stats from the sockets. Signed-off-by: Magnus Karlsson --- include/uapi/linux/if_xdp.h | 7 +++ net/xdp/xsk.c | 42 +- net/xdp/xsk_queue.h | 5 + 3 files changed, 53 insertions(+), 1 deletion(-) 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_RING0 #define XDP_PGOFF_TX_RING 0x8000 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; -- 2.14.1