From: Eric Dumazet
> Sent: 27 May 2020 17:43
> 
> On 5/27/20 8:05 AM, Dmitry Yakunin wrote:
> > This patch adds support of SO_KEEPALIVE flag and TCP related options
> > to bpf_setsockopt() routine. This is helpful if we want to enable or tune
> > TCP keepalive for applications which don't do it in the userspace code.
> > In order to avoid copy-paste, common code from classic setsockopt was moved
> > to auxiliary functions in the headers.
> 
> 
> Please split this in two patches :
> - one adding the helpers, a pure TCP patch.
> - one for BPF additions.
> 
...
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index a6fc234..1035e43 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
...
> > +                   case TCP_KEEPIDLE:
> > +                           if (val < 1 || val > MAX_TCP_KEEPIDLE)
> > +                                   ret = -EINVAL;
> > +                           else
> > +                                   keepalive_time_set(tp, val);
> > +                           break;
> > +                   case TCP_KEEPINTVL:
> > +                           if (val < 1 || val > MAX_TCP_KEEPINTVL)
> > +                                   ret = -EINVAL;
> > +                           else
> > +                                   tp->keepalive_intvl = val * HZ;
> > +                           break;
> > +                   case TCP_KEEPCNT:
> > +                           if (val < 1 || val > MAX_TCP_KEEPCNT)
> > +                                   ret = -EINVAL;
> > +                           else
> > +                                   tp->keepalive_probes = val;
> > +                           break;
> > +                   case TCP_SYNCNT:
> > +                           if (val < 1 || val > MAX_TCP_SYNCNT)
> > +                                   ret = -EINVAL;
> > +                           else
> > +                                   icsk->icsk_syn_retries = val;
> > +                           break;
> > +                   case TCP_USER_TIMEOUT:
> > +                           if (val < 0)
> > +                                   ret = -EINVAL;
> > +                           else
> > +                                   icsk->icsk_user_timeout = val;
> > +                           break;

It also cannot be right to be layer breaking like this
and directly accessing the protocol socket internals.

At least a kernel_setsockopt() function keeps this separate
and ensures that the socket type is correct.

        David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

Reply via email to