From: Antonio Quartulli <antonio@open-mesh.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bridge@lists.linux-foundation.org"
<bridge@lists.linux-foundation.org>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [Bridge] [RFC net] bridge: clean the nf_bridge status when forwarding the skb
Date: Fri, 27 Sep 2013 00:01:43 +0200 [thread overview]
Message-ID: <20130926220143.GC1228@open-mesh.com> (raw)
In-Reply-To: <20130926143248.2799b4ea@nehalam.linuxnetplumber.net>
[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]
On Thu, Sep 26, 2013 at 02:32:48PM -0700, Stephen Hemminger wrote:
> > > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> > > > index 4b81b14..65864bc 100644
> > > > --- a/net/bridge/br_forward.c
> > > > +++ b/net/bridge/br_forward.c
> > > > @@ -49,6 +49,11 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
> > > > } else {
> > > > skb_push(skb, ETH_HLEN);
> > > > br_drop_fake_rtable(skb);
> > > > +
> > > > + /* clean the NF bridge data */
> > > > + nf_bridge_put(skb->nf_bridge);
> > > > + skb->nf_bridge = NULL;
> > > > +
> > > > dev_queue_xmit(skb);
> > > > }
> > > >
> >
> > Regarding CONFIG_BRIDGE_NETFILTER you are right, thanks.
> >
> > >
> > > I think the header will also be garbage if bridge on bridge with netfilter is used.
> > > See nf_bridge_save_header.
> >
> > What header are you referring to? nf_bridge_save_header() saves the header in
> > skb->nf_bridge->data, which is freed during nf_bridge_put() (assuming
> > ->use reached 0).
> >
> >
>
> If bridge is stacked the original ether header will get overwritten by the second
> call to save_header.
Sorry, but I am not getting what you mean (I am new to the code and it is late here..):
save_header() will store the Ethernet header in nf_bridge->data for
later recover (if needed).
By freeing nf_bridge I also destroy this header copy.
When the skb enters the second bridge, save_header() will save again the header
in nf_bridge->data. But I don't see how this can create a problem.
The problem I had before this patch comes from the fact that
nf_bridge_copy_header() is invoked in the second bridge with the nf_bridge state
of the first. This was overwriting my the packet Ethernet header with what the
first invocation of save_header() stored in nf_bridge->data.
But by unsetting nf_bridge I think I am preventing this from happening again.
no?
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Antonio Quartulli <antonio@open-mesh.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>,
"bridge@lists.linux-foundation.org"
<bridge@lists.linux-foundation.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [RFC net] bridge: clean the nf_bridge status when forwarding the skb
Date: Fri, 27 Sep 2013 00:01:43 +0200 [thread overview]
Message-ID: <20130926220143.GC1228@open-mesh.com> (raw)
In-Reply-To: <20130926143248.2799b4ea@nehalam.linuxnetplumber.net>
[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]
On Thu, Sep 26, 2013 at 02:32:48PM -0700, Stephen Hemminger wrote:
> > > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> > > > index 4b81b14..65864bc 100644
> > > > --- a/net/bridge/br_forward.c
> > > > +++ b/net/bridge/br_forward.c
> > > > @@ -49,6 +49,11 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
> > > > } else {
> > > > skb_push(skb, ETH_HLEN);
> > > > br_drop_fake_rtable(skb);
> > > > +
> > > > + /* clean the NF bridge data */
> > > > + nf_bridge_put(skb->nf_bridge);
> > > > + skb->nf_bridge = NULL;
> > > > +
> > > > dev_queue_xmit(skb);
> > > > }
> > > >
> >
> > Regarding CONFIG_BRIDGE_NETFILTER you are right, thanks.
> >
> > >
> > > I think the header will also be garbage if bridge on bridge with netfilter is used.
> > > See nf_bridge_save_header.
> >
> > What header are you referring to? nf_bridge_save_header() saves the header in
> > skb->nf_bridge->data, which is freed during nf_bridge_put() (assuming
> > ->use reached 0).
> >
> >
>
> If bridge is stacked the original ether header will get overwritten by the second
> call to save_header.
Sorry, but I am not getting what you mean (I am new to the code and it is late here..):
save_header() will store the Ethernet header in nf_bridge->data for
later recover (if needed).
By freeing nf_bridge I also destroy this header copy.
When the skb enters the second bridge, save_header() will save again the header
in nf_bridge->data. But I don't see how this can create a problem.
The problem I had before this patch comes from the fact that
nf_bridge_copy_header() is invoked in the second bridge with the nf_bridge state
of the first. This was overwriting my the packet Ethernet header with what the
first invocation of save_header() stored in nf_bridge->data.
But by unsetting nf_bridge I think I am preventing this from happening again.
no?
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2013-09-26 22:01 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-26 20:19 [Bridge] [RFC net] bridge: clean the nf_bridge status when forwarding the skb Antonio Quartulli
2013-09-26 20:19 ` Antonio Quartulli
2013-09-26 21:08 ` [Bridge] " Stephen Hemminger
2013-09-26 21:08 ` Stephen Hemminger
2013-09-26 21:10 ` [Bridge] " Stephen Hemminger
2013-09-26 21:10 ` Stephen Hemminger
2013-09-26 21:16 ` [Bridge] " Antonio Quartulli
2013-09-26 21:16 ` Antonio Quartulli
2013-09-26 21:32 ` [Bridge] " Stephen Hemminger
2013-09-26 21:32 ` Stephen Hemminger
2013-09-26 22:01 ` Antonio Quartulli [this message]
2013-09-26 22:01 ` Antonio Quartulli
2013-10-14 22:20 ` [Bridge] " Antonio Quartulli
2013-10-14 22:20 ` Antonio Quartulli
2013-10-14 22:27 ` [Bridge] " Stephen Hemminger
2013-10-14 22:27 ` Stephen Hemminger
2013-10-14 22:35 ` [Bridge] " Antonio Quartulli
2013-10-14 22:35 ` Antonio Quartulli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130926220143.GC1228@open-mesh.com \
--to=antonio@open-mesh.com \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.