On Fri, May 21, 2021 at 3:09 PM John Fastabend [off-list ref] wrote:

OK either add the counters in this patch or as a series of two
patches so we get a complete fix in one series. Without it some
box out there will randomly drop UDP packets, probably DNS
packets for extra fun, and we will have no way of knowing other
than sporadic packet loss. Unless your arguing against having the
counters or that the counters don't make sense for some reason?

I never object increasing any counter here, My argument is it
belongs to a separate patch for 3 reasons:

1) TCP does not have one either, hence needs to fix together;

2) A patch should fix one bug, not two or more bugs together;

3) It is not the only one place which needs to increase the
counter, all of these kfree_skb()'s need, for example, this one
inside sk_psock_verdict_recv():

        psock = sk_psock(sk);
        if (unlikely(!psock)) {
                len = 0;
                kfree_skb(skb);
                goto out;
        }

This example also shows it is harder to do so, because
sk_psock_verdict_recv() is independent of any protocol, it is
hard to increase a protocol-specific counter there.

(Another one is in sk_psock_verdict_apply().)

quoted

counters either, yet another reason it deserves a separate patch
to address both.

TCP case is different if we drop packets in a TCP error case
thats not a 'lets increment the counters' problem the drop needs
to be fixed we can't let data fall out of a TCP stream because
no one will retransmit it. We've learned this the hard way.

I think TCP always increases some counter when dropping
a packet despite of retransmission, for example:

static void tcp_drop(struct sock *sk, struct sk_buff *skb)
{
        sk_drops_add(sk, skb);
        __kfree_skb(skb);
}

Thanks.