Message ID | 1413568169-4123-1-git-send-email-xiyou.wangcong@gmail.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2014-10-17 at 10:49 -0700, Cong Wang wrote: > From: Cong Wang <cwang@twopensource.com> > > Probably not a big deal, but IP is not the only network protocol, > don't clear skb->cb just for IP. > > Also, IPv6 header is not always defined in struct tcp_skb_cb. > > Cc: Krzysztof Kolasa <kkolasa@winsoft.pl> > Cc: Eric Dumazet <edumazet@google.com> > Signed-off-by: Cong Wang <cwang@twopensource.com> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > --- > net/ipv4/tcp_output.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c > index e13d778..ee356e5 100644 > --- a/net/ipv4/tcp_output.c > +++ b/net/ipv4/tcp_output.c > @@ -1005,9 +1005,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, > /* Our usage of tstamp should remain private */ > skb->tstamp.tv64 = 0; > > - /* Cleanup our debris for IP stacks */ > - memset(skb->cb, 0, max(sizeof(struct inet_skb_parm), > - sizeof(struct inet6_skb_parm))); > + memset(TCP_SKB_CB(skb), 0, sizeof(*TCP_SKB_CB(skb))); > > err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl); > Usually, each layer is responsible for clearing skb->cb[] at its entry point. Or more exactly it does not care of previous garbage. There is no evidence your patch is needed. I was maybe too defensive when I added this, because I wanted to make only TCP changes. We should instead remove the memset() in TCP and fix IP/IPv6 if necessary. But this should wait net-next being open. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 17, 2014 at 11:18 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Fri, 2014-10-17 at 10:49 -0700, Cong Wang wrote: >> From: Cong Wang <cwang@twopensource.com> >> >> Probably not a big deal, but IP is not the only network protocol, >> don't clear skb->cb just for IP. >> >> Also, IPv6 header is not always defined in struct tcp_skb_cb. >> >> Cc: Krzysztof Kolasa <kkolasa@winsoft.pl> >> Cc: Eric Dumazet <edumazet@google.com> >> Signed-off-by: Cong Wang <cwang@twopensource.com> >> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> >> --- >> net/ipv4/tcp_output.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c >> index e13d778..ee356e5 100644 >> --- a/net/ipv4/tcp_output.c >> +++ b/net/ipv4/tcp_output.c >> @@ -1005,9 +1005,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, >> /* Our usage of tstamp should remain private */ >> skb->tstamp.tv64 = 0; >> >> - /* Cleanup our debris for IP stacks */ >> - memset(skb->cb, 0, max(sizeof(struct inet_skb_parm), >> - sizeof(struct inet6_skb_parm))); >> + memset(TCP_SKB_CB(skb), 0, sizeof(*TCP_SKB_CB(skb))); >> >> err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl); >> > > > Usually, each layer is responsible for clearing skb->cb[] at its entry > point. Or more exactly it does not care of previous garbage. > > There is no evidence your patch is needed. This is why I said "probably not a big deal" in changelog, I never say it fixes anything, just a cleanup. > > I was maybe too defensive when I added this, because I wanted to make > only TCP changes. > > We should instead remove the memset() in TCP and fix IP/IPv6 if > necessary. That works too and sounds better. > > But this should wait net-next being open. > > Yeah, agreed, I will update and resend when net-next is reopen. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index e13d778..ee356e5 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -1005,9 +1005,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, /* Our usage of tstamp should remain private */ skb->tstamp.tv64 = 0; - /* Cleanup our debris for IP stacks */ - memset(skb->cb, 0, max(sizeof(struct inet_skb_parm), - sizeof(struct inet6_skb_parm))); + memset(TCP_SKB_CB(skb), 0, sizeof(*TCP_SKB_CB(skb))); err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl);