[PATCH 0/3] bridge: implement restricted forwarding policy

[PATCH 0/3] bridge: implement restricted forwarding policy

[Antonio Quartulli]

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

[PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

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@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

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Stephen Hemminger]

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@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@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
>

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2509 bytes --]

Hi Stephen,

thank you for your reply.

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.
> 

The point is that netfilter would not help me in "distributing" this policy
remotely over a generic layer2 network.

Using these flags, instead, I can make other modules (e.g. batman-adv) notice
that the skb has been marked and then react using their own logic.

If netfilter (at the bridge level) could "mark" the skbs somehow then I could use
it for this purpose. But I don't think this is really possible.

Cheers,

> On Mon, Apr 8, 2013 at 10:41 AM, Antonio Quartulli
> <antonio@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@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
> >

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 3152 bytes --]

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 :)

Thanks,

> 
> On Mon, Apr 8, 2013 at 10:41 AM, Antonio Quartulli
> <antonio@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@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
> >

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Jamal Hadi Salim]

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.

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 :)
>

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Jamal Hadi Salim]

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

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Stephen Hemminger]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Wed, 10 Apr 2013 18:54:34 +0200
Antonio Quartulli <antonio@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...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

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@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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] if.h: add IFF_BRIDGE_RESTRICTED flag

[Jamal Hadi Salim]

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,
>

[PATCH 2/3] sk_buff: add bridge_restricted flag

[Antonio Quartulli]

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@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

[PATCH 3/3] bridge: implement restricted port forwarding policy

[Antonio Quartulli]

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@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

Using skb->mark outside netfilter (was: [PATCH 0/3] bridge: implement restricted forwarding policy)

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]

Hello all,

On Mon, Apr 08, 2013 at 10:41:00 -0700, Antonio Quartulli wrote:
[...]

> 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.
> 

I'm resurrecting this thread because after Jamal's suggestion I decided to
finally drop the idea of introducing such a net flag and focus on a different
approach.

The new idea is to use tc to mark packets arriving into the bridge
through a predefined port (chosen by the admin when setting up the tc rule) and
again use tc to drop packets having such mark when they are going out through
another predefined port.

Now to extend this mechanism network-wide (remember that the use case is a Layer2
mesh network set up with batman-adv) I'm going to introduce a mechanism in
batman-adv itself which is supposed to read and write the skb->mark field
so that the value contained when the packet is leaving one end can be restored
later on the other end of the intra-mesh communication (only if it matches a pre
configured one).
This would allow the remote node to perform the same filtering
operation as if the packet was locally generated.

Now my question is (I think David is the one who should probably decide here):
is batman-adv allowed to touch the mark field of the sk_buff structure? Or is it
reserved for netfilter purposes only?

In my opinion this field (which survives whatever path the skb follows) is the
best option for this task as it allows batman-adv to "communicate" with
tc and possibly netfilter itself (if in the future users would like to perform
more complicated operations).


To clarify the idea, here you have an ascii art representing a possible setup
and how the mark will be read and set:


Scenario:
=========
- Two mesh nodes configured the same way.
- Packets arriving via eth0 have to be marked with M.
- Packets marked with M should be dropped before going out through eth1.


           +------+
     ------|bridge|----
    |      +------+    |
    |         |        |
   [M]skb     |       [M]skb
 +------+ +------+ +------+     The skb is generated when arriving on eth0 and
 | bat0 | | eth1 | | eth0 |     is marked with the value M.
 +------+ +------+ +------+     batman-adv (operating in bat0) recognises the
    |                pkt        mark and spread the information in the mesh
 +------+                       (other than forwarding the packet to the mesh).
 |wlan0 |
 +------+
    :bat{pkt}
 ___:_______
/           \
|   MESH    |
\___________/
    :
    :bat{pkt}
 +------+                        batman-adv on the receiving side knows that
 |wlan0 |                        this skb has to be marked and so it writes M
 +------+                        into skb->mark.
    |[]skb		           The skb is now marked and can be dropped as
 +------+ +------+ +------+      instructed.
 | bat0 | | eth1 | | eth0 |
 +------+ +------+ +------+
   [M]skb     |        |
    |         |        |
    |     +------+     |
    ------|bridge|-----
          +------+



Cheers,




-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Using skb->mark outside netfilter

[Jamal Hadi Salim]

On 13-05-06 02:48 PM, Antonio Quartulli wrote:

>
> Now to extend this mechanism network-wide (remember that the use case is a Layer2
> mesh network set up with batman-adv) I'm going to introduce a mechanism in
> batman-adv itself which is supposed to read and write the skb->mark field
> so that the value contained when the packet is leaving one end can be restored
> later on the other end of the intra-mesh communication (only if it matches a pre
> configured one).
> This would allow the remote node to perform the same filtering
> operation as if the packet was locally generated.
>

Nice idea.
How do you encode the mark in the batman header?

> Now my question is (I think David is the one who should probably decide here):
> is batman-adv allowed to touch the mark field of the sk_buff structure? Or is it
> reserved for netfilter purposes only?
>


Ok, let me see if i can come up with a definition for mark:
"A general purpose 32 bit tag used in the network subsystem
to carry metadata with global implications across network
sub-subsystem. As an example, a driver could mark on incoming
packet to be used by the ingress tc classifier-action sub-subsystem,
netfilter, ipsec all to execute provisioned policies. Since marks
have global implications, any changes in the datapath flow would
have repurcassions at each sub-subsytem level the packet flows to
next.
The mark could also be used to direct packets to matching socket
apps (eg tcp applications) which have registered for it ......
... more text here on how the mark is used for egress ...
"

So please, use it.

> In my opinion this field (which survives whatever path the skb follows) is the
> best option for this task as it allows batman-adv to "communicate" with
> tc and possibly netfilter itself (if in the future users would like to perform
> more complicated operations).
>

Yes. Also apps which setsockopt to it etc

>
> To clarify the idea, here you have an ascii art representing a possible setup
> and how the mark will be read and set:

Looks sane to me.

cheers,
jamal

Re: Using skb->mark outside netfilter

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

Hi Jamal,

On Tue, May 07, 2013 at 06:04:18 -0700, Jamal Hadi Salim wrote:
> On 13-05-06 02:48 PM, Antonio Quartulli wrote:
> 
> >
> > Now to extend this mechanism network-wide (remember that the use case is a Layer2
> > mesh network set up with batman-adv) I'm going to introduce a mechanism in
> > batman-adv itself which is supposed to read and write the skb->mark field
> > so that the value contained when the packet is leaving one end can be restored
> > later on the other end of the intra-mesh communication (only if it matches a pre
> > configured one).
> > This would allow the remote node to perform the same filtering
> > operation as if the packet was locally generated.
> >
> 
> Nice idea.
> How do you encode the mark in the batman header?

The mark is not really encoded in the batman header.

Each node is configured with a mark value (the values have to be the same on
everynode in order to make the mechanism work correctly), then batman-adv will
advertise to the rest of the mesh which host is sending marked packets.

Receiving nodes will then restore the mark in the skb each time it is coming from
one of those advertised hosts.

A future feature may consists in carrying the mark directly into the header so
that batman-adv itself does not have to take care about the meaning of such
value but will just carry it (we still have to think about it...it is just an
idea now)

> >
> > To clarify the idea, here you have an ascii art representing a possible setup
> > and how the mark will be read and set:
> 
> Looks sane to me.
> 

Thanks a lot!
I will use it :)


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Using skb->mark outside netfilter

[Jamal Hadi Salim]

On 13-05-07 09:23 AM, Antonio Quartulli wrote:

> The mark is not really encoded in the batman header.
>
> Each node is configured with a mark value (the values have to be the same on
> everynode in order to make the mechanism work correctly), then batman-adv will
> advertise to the rest of the mesh which host is sending marked packets.
>
[..]
> Receiving nodes will then restore the mark in the skb each time it is coming from
> one of those advertised hosts.
>

I see - so some control "protocol" will be used to advertise the mark
and which host will use such a mark? Or are you saying some admin is
going around and configuring 100 batman nodes? ;->

Note: I have no freaking clue in regards to batman or its usage.

> A future feature may consists in carrying the mark directly into the header so
> that batman-adv itself does not have to take care about the meaning of such
> value but will just carry it (we still have to think about it...it is just an
> idea now)
>

I think that may be more interesting because it may allow you to use
many different marks with many different meanings. It doesnt disqualify
a control protocol advertising the mark.

cheers,
jamal

Re: Using skb->mark outside netfilter

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2530 bytes --]

On Tue, May 07, 2013 at 06:30:04 -0700, Jamal Hadi Salim wrote:
> On 13-05-07 09:23 AM, Antonio Quartulli wrote:
> 
> > The mark is not really encoded in the batman header.
> >
> > Each node is configured with a mark value (the values have to be the same on
> > everynode in order to make the mechanism work correctly), then batman-adv will
> > advertise to the rest of the mesh which host is sending marked packets.
> >
> [..]
> > Receiving nodes will then restore the mark in the skb each time it is coming from
> > one of those advertised hosts.
> >
> 
> I see - so some control "protocol" will be used to advertise the mark
> and which host will use such a mark? Or are you saying some admin is
> going around and configuring 100 batman nodes? ;->

The mark is locally configured. So no control protocol that advertised it.
It will be a new config option of batman-adv, like others that it already has.

What you are talking about is a common config problem in a mesh network:
whenever you have to change something you have to do it on each and every
node..we can't do anything about that :) Everybody uses is own solution for this.

However the mark is only important locally, because no matter what mark a node
will use, the important part is that batman-adv and tc are using the same (on
the same node).


By the way we will have soon a userspace daemon (namely "alfred") which can be
used to spread "external" information over the mesh and this mark may be one of
them (if we really wanted to coordinate it, but it is not necessary as I
explained before).


> 
> Note: I have no freaking clue in regards to batman or its usage.
> 

no problem :)

> > A future feature may consists in carrying the mark directly into the header so
> > that batman-adv itself does not have to take care about the meaning of such
> > value but will just carry it (we still have to think about it...it is just an
> > idea now)
> >
> 
> I think that may be more interesting because it may allow you to use
> many different marks with many different meanings. It doesnt disqualify
> a control protocol advertising the mark.
> 

Yeah that would really be interesting.
Maybe we will implement it as soon as more uses cases will pop up. Actually this
simple distributed access control is the only purpose we have in mind at the
moment :)

Thank you very much for your valuable feedbacks!

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]