Messages in this thread | | | Date | Mon, 7 Feb 2022 21:03:27 -0800 | Subject | Re: [PATCH 2/2] net: tun: track dropped skb via kfree_skb_reason() | From | David Ahern <> |
| |
On 2/7/22 7:55 PM, Dongli Zhang wrote: > The TUN can be used as vhost-net backend. E.g, the tun_net_xmit() is the > interface to forward the skb from TUN to vhost-net/virtio-net. > > However, there are many "goto drop" in the TUN driver. Therefore, the > kfree_skb_reason() is involved at each "goto drop" to help userspace > ftrace/ebpf to track the reason for the loss of packets. > > Cc: Joao Martins <joao.m.martins@oracle.com> > Cc: Joe Jin <joe.jin@oracle.com> > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> > --- > drivers/net/tun.c | 33 +++++++++++++++++++++++++-------- > include/linux/skbuff.h | 6 ++++++ > include/trace/events/skb.h | 6 ++++++ > 3 files changed, 37 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index fed85447701a..d67f2419dbb4 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1062,13 +1062,16 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev) > struct netdev_queue *queue; > struct tun_file *tfile; > int len = skb->len; > + int drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> > rcu_read_lock(); > tfile = rcu_dereference(tun->tfiles[txq]); > > /* Drop packet if interface is not attached */ > - if (!tfile) > + if (!tfile) { > + drop_reason = SKB_DROP_REASON_DEV_NOT_ATTACHED;
That is going to be a confusing reason code (tap device existed to get here) and does not really explain this error.
> goto drop; > + } > > if (!rcu_dereference(tun->steering_prog)) > tun_automq_xmit(tun, skb); > @@ -1078,19 +1081,27 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev) > /* Drop if the filter does not like it. > * This is a noop if the filter is disabled. > * Filter can be enabled only for the TAP devices. */ > - if (!check_filter(&tun->txflt, skb)) > + if (!check_filter(&tun->txflt, skb)) { > + drop_reason = SKB_DROP_REASON_TAP_RUN_FILTER;
just SKB_DROP_REASON_TAP_FILTER
> goto drop; > + } > > if (tfile->socket.sk->sk_filter && > - sk_filter(tfile->socket.sk, skb)) > + sk_filter(tfile->socket.sk, skb)) { > + drop_reason = SKB_DROP_REASON_SKB_TRIM;
SKB_DROP_REASON_SOCKET_FILTER
The remainder of your changes feels like another variant of your previous "function / line" reason code. You are creating new reason codes for every goto failure with a code based name. The reason needs to be the essence of the failure in a user friendly label.
| |