[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200731173734.2eqvydx4cb5lllty@kafai-mbp.dhcp.thefacebook.com>
Date: Fri, 31 Jul 2020 10:37:34 -0700
From: Martin KaFai Lau <kafai@...com>
To: Eric Dumazet <edumazet@...gle.com>
CC: bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
kernel-team <kernel-team@...com>,
Lawrence Brakmo <brakmo@...com>,
Neal Cardwell <ncardwell@...gle.com>,
netdev <netdev@...r.kernel.org>,
Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH v3 bpf-next 4/9] tcp: Add unknown_opt arg to
tcp_parse_options
On Fri, Jul 31, 2020 at 09:12:10AM -0700, Eric Dumazet wrote:
> On Thu, Jul 30, 2020 at 1:58 PM Martin KaFai Lau <kafai@...com> wrote:
> >
> > In the latter patch, the bpf prog only wants to be called to handle
> > a header option if that particular header option cannot be handled by
> > the kernel. This unknown option could be written by the peer's bpf-prog.
> > It could also be a new standard option that the running kernel does not
> > support it while a bpf-prog can handle it.
> >
> > In a latter patch, the bpf prog will be called from tcp_validate_incoming()
> > if there is unknown option and a flag is set in tp->bpf_sock_ops_cb_flags.
> >
> > Instead of using skb->cb[] in an earlier attempt, this patch
> > adds an optional arg "bool *unknown_opt" to tcp_parse_options().
> > The bool will be set to true if it has encountered an option
> > that the kernel does not recognize.
> >
> > Signed-off-by: Martin KaFai Lau <kafai@...com>
> > ---
> > drivers/infiniband/hw/cxgb4/cm.c | 2 +-
> > include/net/tcp.h | 3 ++-
> > net/ipv4/syncookies.c | 2 +-
> > net/ipv4/tcp_input.c | 40 +++++++++++++++++++++-----------
> > net/ipv4/tcp_minisocks.c | 4 ++--
> > net/ipv6/syncookies.c | 2 +-
> > 6 files changed, 34 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
> > index 30e08bcc9afb..dedca6576bb9 100644
> > --- a/drivers/infiniband/hw/cxgb4/cm.c
> > +++ b/drivers/infiniband/hw/cxgb4/cm.c
> > @@ -3949,7 +3949,7 @@ static void build_cpl_pass_accept_req(struct sk_buff *skb, int stid , u8 tos)
> > */
> > memset(&tmp_opt, 0, sizeof(tmp_opt));
> > tcp_clear_options(&tmp_opt);
> > - tcp_parse_options(&init_net, skb, &tmp_opt, 0, NULL);
> > + tcp_parse_options(&init_net, skb, &tmp_opt, 0, NULL, NULL);
> >
> > req = __skb_push(skb, sizeof(*req));
> > memset(req, 0, sizeof(*req));
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 895e7aabf136..d49d8f1c961a 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -413,7 +413,8 @@ int tcp_mmap(struct file *file, struct socket *sock,
> > #endif
> > void tcp_parse_options(const struct net *net, const struct sk_buff *skb,
> > struct tcp_options_received *opt_rx,
> > - int estab, struct tcp_fastopen_cookie *foc);
> > + int estab, struct tcp_fastopen_cookie *foc,
> > + bool *unknown_opt);
> > const u8 *tcp_parse_md5sig_option(const struct tcphdr *th);
> >
>
> Instead of changing signatures of many functions (and make future
> stable backports challenging)
> how about adding a field into 'struct tcp_options_received' ?
Sounds good. There is a one byte hole in 'struct tcp_options_received',
so it won't matter much even there is "rx_opt" in "struct tcp_sock".
Powered by blists - more mailing lists