Cong Wang wrote: > On Thu, May 27, 2021 at 10:27 PM John Fastabend > <john.fastabend@xxxxxxxxx> wrote: > > > > Cong Wang wrote: > > > From: Cong Wang <cong.wang@xxxxxxxxxxxxx> > > > > > > It is hard to observe packet drops without increasing relevant > > > drop counters, here we should increase sk->sk_drops which is > > > a protocol-independent counter. Fortunately psock is always > > > associated with a struct sock, we can just use psock->sk. > > > > > > Suggested-by: John Fastabend <john.fastabend@xxxxxxxxx> > > > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > > > Cc: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> > > > Cc: Lorenz Bauer <lmb@xxxxxxxxxxxxxx> > > > Signed-off-by: Cong Wang <cong.wang@xxxxxxxxxxxxx> > > > --- > > > net/core/skmsg.c | 22 ++++++++++++++-------- > > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > > > > [...] > > > > > @@ -942,7 +948,7 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb, > > > case __SK_DROP: > > > default: > > > out_free: > > > - kfree_skb(skb); > > > + sock_drop(psock->sk, skb); > > > > I must have missed this on first review. > > > > Why should we mark a packet we intentionally drop as sk_drops? I think > > we should leave it as just kfree_skb() this way sk_drops is just > > the error cases and if users want this counter they can always add > > it to the bpf prog itself. > > This is actually a mixed case of error and non-error drops, > because bpf_sk_redirect_map() could return SK_DROP > in error cases. And of course users could want to drop packets > in whatever cases. > > But if you look at packet filter cases, for example UDP one, > it increases drop counters too when user-defined rules drop > them: > > 2182 if (sk_filter_trim_cap(sk, skb, sizeof(struct udphdr))) > 2183 goto drop; > 2184 > ... > 2192 drop: > 2193 __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); > 2194 atomic_inc(&sk->sk_drops); > 2195 kfree_skb(skb); > 2196 return -1; > > > Thanks. OK same for TCP side for sk_filter_trim_cap() case. Works for me.
