Linux Ethernet Bridging - Apr 2013 - [Bridge] [PATCH 0/3] bridge: implement restricted forwarding policy

Hello,

with this patchset I would like to introduce a new feature to be added to the
bridge module that is later going to be one of the building block of a
distributed technique implemented using batman-adv.

The scenario where we want to play is a Layer2 mesh network handled by
the batman-adv module. For who may not know how it works, batman-adv creates a
sort of "extended broadcast domain" shared among nodes that are not
directly
connected to each other.
This mechanism provides every node the view of the batman-adv network
as it was a very big Ethernet switch.


The goal we want to achieve by adding this feature consists in selecting a
subset of the interfaces available on all the hosts participating to the mesh
network and to prevent packets from being forwarded from one of them to another.

Looking again at the "very big switch" abstraction this would allow a
mesh
network administrator to define a sort of "not forwarding policy network
wide"
between some of the big switch's port.

This patchset is introducing the small piece needed by every hosts in the
network to handle the policy locally. In particular, I'm adding a new flag
for
the net_device->flags member (IFF_BRIDGE_RESTRICTED) and a new attribute to
the
sk_buff structure (bridge_restricted), then the bridge code is modified to
obey the following rule:

* do not forward any skb with the bridge_restricted attribute set to interfaces
  marked with the IFF_BRIDGE_RESTRICTED flag.


Later, a change to batman-adv will follow which will make it spit out skbs with
the bridge_restricted member set depending on its distributed logic.


I am not entirely sure if I chose the very best place for those flags. If not,
please advise :)

Thanks a lot.


Cheers,


Antonio Quartulli (3):
  if.h: add IFF_BRIDGE_RESTRICTED flag
  sk_buff: add bridge_restricted flag
  bridge: implement restricted port forwarding policy

 include/linux/skbuff.h  |  3 ++-
 include/uapi/linux/if.h |  1 +
 net/bridge/br_forward.c | 18 +++++++++++++++++-
 net/bridge/br_input.c   |  6 ++++++
 net/core/dev.c          |  2 +-
 5 files changed, 27 insertions(+), 3 deletions(-)

-- 
1.8.1.5

This new flag tells whether a network device has to be
considered as restricted in the new bridge forwarding logic.

Signed-off-by: Antonio Quartulli <antonio at open-mesh.com>
---
 include/uapi/linux/if.h | 1 +
 net/core/dev.c          | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 1ec407b..5c3a9bd 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -83,6 +83,7 @@
 #define IFF_SUPP_NOFCS	0x80000		/* device supports sending custom FCS */
 #define IFF_LIVE_ADDR_CHANGE 0x100000	/* device supports hardware address
 					 * change when it's running */
+#define IFF_BRIDGE_RESTRICTED 0x200000	/* device is bridge-restricted */
 
 
 #define IF_GET_IFACE	0x0001		/* for querying only */
diff --git a/net/core/dev.c b/net/core/dev.c
index 3655ff9..49eafc8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4627,7 +4627,7 @@ int __dev_change_flags(struct net_device *dev, unsigned
int flags)
 
 	dev->flags = (flags & (IFF_DEBUG | IFF_NOTRAILERS | IFF_NOARP |
 			       IFF_DYNAMIC | IFF_MULTICAST | IFF_PORTSEL |
-			       IFF_AUTOMEDIA)) |
+			       IFF_AUTOMEDIA | IFF_BRIDGE_RESTRICTED)) |
 		     (dev->flags & (IFF_UP | IFF_VOLATILE | IFF_PROMISC |
 				    IFF_ALLMULTI));
 
-- 
1.8.1.5

This new flag tells whether the skb has been originated by
an interface marked with the IFF_BRIDGE_RESTRICTED flag.

This information can be used to prevent a packet from being
forwarded to another restricted interface.

Signed-off-by: Antonio Quartulli <antonio at open-mesh.com>
---
 include/linux/skbuff.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e27d1c7..2054073 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -492,7 +492,8 @@ struct sk_buff {
 	 * headers if needed
 	 */
 	__u8			encapsulation:1;
-	/* 7/9 bit hole (depending on ndisc_nodetype presence) */
+	__u8			bridge_restricted:1;
+	/* 9/11 bit hole (depending on ndisc_nodetype presence) */
 	kmemcheck_bitfield_end(flags2);
 
 #ifdef CONFIG_NET_DMA
-- 
1.8.1.5

Prevent an skb with the bridge_restricted attribute set from
being forwarded to devices marked with the
IFF_BRIDGE_RESTRICTED flag.

Signed-off-by: Antonio Quartulli <antonio at open-mesh.com>
---
 net/bridge/br_forward.c | 18 +++++++++++++++++-
 net/bridge/br_input.c   |  6 ++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 092b20e..52b9031 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -26,13 +26,29 @@ static int deliver_clone(const struct net_bridge_port *prev,
 			 void (*__packet_hook)(const struct net_bridge_port *p,
 					       struct sk_buff *skb));
 
+/**
+ * should_restrict - check if incoming and outgoing ports are restricted
+ * @skb: the incoming packet
+ * @dev: the outgoing port
+ *
+ * Return true if the skb is coming from a restricted port and out_dev is
+ * restricted too. False otherwise
+ */
+static inline int should_restrict(const struct sk_buff *skb,
+				  const struct net_device *out_dev)
+{
+	return skb->bridge_restricted &&
+	       (out_dev->flags & IFF_BRIDGE_RESTRICTED);
+}
+
 /* Don't forward packets to originating port or forwarding diasabled */
 static inline int should_deliver(const struct net_bridge_port *p,
 				 const struct sk_buff *skb)
 {
 	return (((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev)
&&
 		br_allowed_egress(p->br, nbp_get_vlan_info(p), skb) &&
-		p->state == BR_STATE_FORWARDING);
+		p->state == BR_STATE_FORWARDING &&
+		!should_restrict(skb, p->dev));
 }
 
 static inline unsigned int packet_length(const struct sk_buff *skb)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 828e2bc..73b4d4d 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -169,6 +169,12 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 
 	p = br_port_get_rcu(skb->dev);
 
+	/* if this skb has been generated by a restricted port, mark it as well
+	 * so that a "remote bridge" does not forward it to another
restricted
+	 * device
+	 */
+	skb->bridge_restricted = !!(skb->dev->flags &
IFF_BRIDGE_RESTRICTED);
+
 	if (unlikely(is_link_local_ether_addr(dest))) {
 		/*
 		 * See IEEE 802.1D Table 7-10 Reserved addresses
-- 
1.8.1.5

The standard way to do this is to use netfilter. Considering the
additional device flags and skb flag changes, I am not sure that your
method is better.

On Mon, Apr 8, 2013 at 10:41 AM, Antonio Quartulli
<antonio at open-mesh.com> wrote:
> This new flag tells whether a network device has to be
> considered as restricted in the new bridge forwarding logic.
>
> Signed-off-by: Antonio Quartulli <antonio at open-mesh.com>
> ---
>  include/uapi/linux/if.h | 1 +
>  net/core/dev.c          | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> index 1ec407b..5c3a9bd 100644
> --- a/include/uapi/linux/if.h
> +++ b/include/uapi/linux/if.h
> @@ -83,6 +83,7 @@
>  #define IFF_SUPP_NOFCS 0x80000         /* device supports sending custom
FCS */
>  #define IFF_LIVE_ADDR_CHANGE 0x100000  /* device supports hardware address
>                                          * change when it's running */
> +#define IFF_BRIDGE_RESTRICTED 0x200000 /* device is bridge-restricted */
>
>
>  #define IF_GET_IFACE   0x0001          /* for querying only */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3655ff9..49eafc8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4627,7 +4627,7 @@ int __dev_change_flags(struct net_device *dev,
unsigned int flags)
>
>         dev->flags = (flags & (IFF_DEBUG | IFF_NOTRAILERS |
IFF_NOARP |
>                                IFF_DYNAMIC | IFF_MULTICAST | IFF_PORTSEL |
> -                              IFF_AUTOMEDIA)) |
> +                              IFF_AUTOMEDIA | IFF_BRIDGE_RESTRICTED)) |
>                      (dev->flags & (IFF_UP | IFF_VOLATILE |
IFF_PROMISC |
>                                     IFF_ALLMULTI));
>
> --
> 1.8.1.5
>

On Tue, Apr 09, 2013 at 05:57:45 -0700, Jamal Hadi Salim
wrote:
> Hi,
> 
> Consider using tc for this.
> You can tag the packet using skb mark on the receiving end point,
> match them on the bridge and execute actions not to forward them.

Does this work at the bridge level? A packet entering a port and going out from
another one can be affected by tc/mark?

> 
> cheers,
> jamal
> 
> On 13-04-09 03:56 AM, Antonio Quartulli wrote:
> > On Mon, Apr 08, 2013 at 11:58:48 -0700, Stephen Hemminger wrote:
> >> The standard way to do this is to use netfilter. Considering the
> >> additional device flags and skb flag changes, I am not sure that
your
> >> method is better.
> >
> > To make it a bit more clear:
> >
> > 1) the skb flag will be used on the "receiving end-point" by
batman-adv to mark
> > received packets and so instruct the bridge to do not forward them to
restricted
> > interfaces.
> >
> > 2) the IFF_ flag is used by batman-adv on the "sending side"
to determine
> > whether a packet has been originated by a restricted interface and so
instruct
> > the remote endpoint to mark the skb when received.
> >
> > 3) to make the bridge code general enough, I decided to let it mark
packets
> > coming from restricted interfaces as well so that it can also apply
the policy
> > at 1) locally, without any further setting. The logic described in 1)
is
> > therefore applied by the bridge even for local packets (not passing
through
> > batman-adv)
> >
> >
> >
> > Point 3) is the only one where netfilter might help. But using two
mechanism to
> > achieve one goal looked not sane to me and therefore I decided to to
do it this
> > way. And actually the code allowing point 3 is only:
> >
> > +       skb->bridge_restricted = !!(skb->dev->flags &
IFF_BRIDGE_RESTRICTED);
> >
> >
> > I hope this summary did not create further confusion :)
> >
> 
-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20130409/bd9b249c/attachment.sig>

On 13-04-09 09:51 AM, Antonio Quartulli wrote:
>
> Does this work at the bridge level? A packet entering a port and going out
from
> another one can be affected by tc/mark?
Yes of course. And on any construct that looks like a netdev (tunnels etc).

cheers,
jamal

Hi Jamal, all,

On Tue, Apr 09, 2013 at 08:49:17 -0700, Jamal Hadi Salim
wrote:
> On 13-04-09 09:51 AM, Antonio Quartulli wrote:
> 
> >
> > Does this work at the bridge level? A packet entering a port and going
out from
> > another one can be affected by tc/mark?
> 
> Yes of course. And on any construct that looks like a netdev (tunnels etc).
> 
Thanks for your hints. After having struggled a bit I found out how to do it
using ebtables and the mark target :)

Thanks a Lot!

These patches seem to be useless now

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20130410/efa91bf3/attachment-0001.sig>

On Wed, 10 Apr 2013 18:54:34 +0200
Antonio Quartulli <antonio at open-mesh.com> wrote:
> Hi Jamal, all,
> 
> On Tue, Apr 09, 2013 at 08:49:17 -0700, Jamal Hadi Salim wrote:
> > On 13-04-09 09:51 AM, Antonio Quartulli wrote:
> > 
> > >
> > > Does this work at the bridge level? A packet entering a port and
going out from
> > > another one can be affected by tc/mark?
> > 
> > Yes of course. And on any construct that looks like a netdev (tunnels
etc).
> > 
> 
> Thanks for your hints. After having struggled a bit I found out how to do
it
> using ebtables and the mark target :)
> 
> Thanks a Lot!
> 
> These patches seem to be useless now
> 
> Cheers,
> 
Come back again, though. The ebtables method offers more flexibility which can
be a good or bad thing...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20130410/a1a93f16/attachment.sig>

On Wed, Apr 10, 2013 at 01:46:09PM -0700, Stephen Hemminger
wrote:
> On Wed, 10 Apr 2013 18:54:34 +0200
> Antonio Quartulli <antonio at open-mesh.com> wrote:
> 
> > Hi Jamal, all,
> > 
> > On Tue, Apr 09, 2013 at 08:49:17 -0700, Jamal Hadi Salim wrote:
> > > On 13-04-09 09:51 AM, Antonio Quartulli wrote:
> > > 
> > > >
> > > > Does this work at the bridge level? A packet entering a port
and going out from
> > > > another one can be affected by tc/mark?
> > > 
> > > Yes of course. And on any construct that looks like a netdev
(tunnels etc).
> > > 
> > 
> > Thanks for your hints. After having struggled a bit I found out how to
do it
> > using ebtables and the mark target :)
> > 
> > Thanks a Lot!
> > 
> Come back again, though. The ebtables method offers more flexibility which
can
> be a good or bad thing...
I just realised that :)

By installing ebtables (meaning modules + userspace tool) my iperf test result
drops from 81Mbps to 66Mbps: former without, latter with ebtables module
enabled.
I did this test between two devices connected with Fast Ethernet.

I thought that most of the code is in netfilter, so shared with iptables, hence
I expected a reasonable overhead why this is much worse.

Does anybody have a clue about this? I should probably start a new thread on the
netfilter mailing list.

However this problem makes ebtables unusable at all.

Suggestions are welcome :)

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20130411/56f1d28e/attachment.sig>

On 13-04-11 06:56 AM, Antonio Quartulli wrote:
> I just realised that :)
>
> By installing ebtables (meaning modules + userspace tool) my iperf test
result
> drops from 81Mbps to 66Mbps: former without, latter with ebtables module
enabled.
> I did this test between two devices connected with Fast Ethernet.
>

Please try tc like i said earlier ;->

cheers,
jamal
> I thought that most of the code is in netfilter, so shared with iptables,
hence
> I expected a reasonable overhead why this is much worse.
>
> Does anybody have a clue about this? I should probably start a new thread
on the
> netfilter mailing list.
>
> However this problem makes ebtables unusable at all.
>
> Suggestions are welcome :)
>
> Cheers,
>