Messages in this thread | | | Subject | Re: [PATCH] net: Reset skb to network header in neigh_hh_output | From | Eric Dumazet <> | Date | Tue, 25 Oct 2016 17:12:22 -0700 |
| |
On Wed, 2016-10-26 at 01:57 +0200, Abdelrhman Ahmed wrote: > > What is the issue you want to fix exactly ? > > Please describe the use case. > > When netfilter hook uses skb_push to add a specific header between network > header and hardware header. > For the first time(s) before caching hardware header, this header will be > removed / overwritten by hardware header due to resetting to network header. > After using the cached hardware header, this header will be kept as we do not > reset. I think this behavior is inconsistent, so we need to reset in both cases. > > > Otherwise, your fix is in fact adding a critical bug. > > Could you explain more as it's not clear to me? >
Maybe my wording was not good here.
What I intended to say is that the __skb_pull(skb, skb_network_offset(skb)) might not be at the right place.
Look at commit e1f165032c8bade3a6bdf546f8faf61fda4dd01c to find the reason.
> > > ---- On Fri, 07 Oct 2016 23:10:56 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote ---- > > On Fri, 2016-10-07 at 16:14 +0200, Abdelrhman Ahmed wrote: > > > When hardware header is added without using cached one, neigh_resolve_output > > > and neigh_connected_output reset skb to network header before adding it. > > > When cached one is used, neigh_hh_output does not reset the skb to network > > > header. > > > > > > The fix is to reset skb to network header before adding cached hardware header > > > to keep the behavior consistent in all cases. > > > > What is the issue you want to fix exactly ? > > > > Please describe the use case. > > > > I highly suggest you take a look at commit > > > > e1f165032c8bade3a6bdf546f8faf61fda4dd01c > > ("net: Fix skb_under_panic oops in neigh_resolve_output") > > > > Otherwise, your fix is in fact adding a critical bug. > > > > > > >
| |