On 2018/11/30 下午1:29, Toshiaki Makita wrote:
On 2018/11/30 11:40, Jason Wang wrote:
On 2018/11/30 上午10:30, Prashant Bhole wrote:
In tun.c skb->len was accessed while doing stats accounting after a
call to netif_receive_skb. We can not access skb after this call
because buffers may be dropped.
The fix for this bug would be to store skb->len in local variable and
then use it after netif_receive_skb(). IMO using xdp data size for
accounting bytes will be better because input for tun_xdp_one() is
xdp_buff.
Hence this patch:
- fixes a bug by removing skb access after netif_receive_skb()
- uses xdp data size for accounting bytes
...
Fixes: 043d222f93ab ("tuntap: accept an array of XDP buffs through
sendmsg()")
Reviewed-by: Toshiaki Makita <makita.toshi...@lab.ntt.co.jp>
Signed-off-by: Prashant Bhole <bhole_prashant...@lab.ntt.co.jp>
---
drivers/net/tun.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e244f5d7512a..6e388792c0a8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2385,6 +2385,7 @@ static int tun_xdp_one(struct tun_struct *tun,
struct tun_file *tfile,
struct xdp_buff *xdp, int *flush)
{
+ unsigned int datasize = xdp->data_end - xdp->data;
struct tun_xdp_hdr *hdr = xdp->data_hard_start;
struct virtio_net_hdr *gso = &hdr->gso;
struct tun_pcpu_stats *stats;
@@ -2461,7 +2462,7 @@ static int tun_xdp_one(struct tun_struct *tun,
stats = get_cpu_ptr(tun->pcpu_stats);
u64_stats_update_begin(&stats->syncp);
stats->rx_packets++;
- stats->rx_bytes += skb->len;
+ stats->rx_bytes += datasize;
u64_stats_update_end(&stats->syncp);
put_cpu_ptr(stats);
Good catch, but you probably need to calculate the datasize after XDP
processing since it may modify the packet length.
(+CC David Ahern who may be interested in this area.)
I'd rather think we should calculate it before XDP.
I checked several drivers behavior. mlx5, bnxt and qede use hardware
counters for rx bytes, which means the size is calculated before XDP.
nfp calculate it by software but before XDP. On the other hand, intel
drivers use skb->len. So currently bytes counters do not look
consistent, but I think calculation before XDP is more common.
Ok, I'm fine with either. Consider this fixes a real issue we need let
it to be in -net as soon as possible. We can change the behavior of
counter in the future.
So
Acked-by: Jason Wang <jasow...@redhat.com>
According to patchwork status, you probably need to repost.
Thanks
