Re: [Patch bpf v2] skmsg: check sk_rcvbuf limit before queuing to ingress_skb

[Cong Wang <xiyou.wangcong@gmail.com>]

On Thu, Jul 1, 2021 at 11:00 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Thu, Jul 1, 2021 at 9:23 AM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> > We can't just drop the packet in the memory overrun case here. This will
> > break TCP because the data will be gone and no one will retransmit.
> >
> > Thats why in the current scheme on redirect we can push back when we
> > move it to the other queues ingress message queue or redirect into
> > the other socket via send.
> >
> > At one point I considered charging the data sitting in the ingress_skb?
> > Would that solve the problem here? I think it would cause the enqueue
> > at the UDP to start dropping packets from __udp_enqueue_schedule_skb()?
>
> I tried to move skb_set_owner_r() here, TCP is clearly unhappy about it,
> as I explained in changelog. Yes, it probably helps if we could move it here.

Sorry for replying too quickly. Actually it probably does _not_ help,
because we have to limit the dest socket buffer, not the source.

For example, if we have 5 sockets in a sockmap and all of them send
packets to the same one, the dest socket could still get 5x sk_rcvbuf
bytes pending in its ingress_skb.

And I do not know why you want to single out the TCP case, at least
the ebpf program can decide to drop TCP packets too, this drop is
not any different from the drop due to rcvbuf overlimit.

Therefore, I think my patch is fine as it is.

Thanks.