netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] Add some new flags to bridges.
@ 2013-04-29 15:58 Vlad Yasevich
  2013-04-29 15:58 ` [PATCH net-next 1/2] bridge: Add flag to control mac learning Vlad Yasevich
  2013-04-29 15:58 ` [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood Vlad Yasevich
  0 siblings, 2 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-29 15:58 UTC (permalink / raw)
  To: netdev; +Cc: Vlad Yasevich, bridge, mst

The follows series adds 2 new flags to bridge.  One flag allows
the user to control whether mac learning is performed on the interface
or not.  By default mac learning is on.
The other flag allows the user to control whether unicast traffic
is flooded (send without an fdb) to a given unicast port.  Default is
on.

Vlad Yasevich (2):
  bridge: Add flag to control mac learning.
  bridge: Add a flag to control unicast packet flood.

 include/uapi/linux/if_link.h |    2 ++
 net/bridge/br_device.c       |    8 ++++----
 net/bridge/br_fdb.c          |    4 ++++
 net/bridge/br_forward.c      |   14 +++++++++-----
 net/bridge/br_if.c           |    2 +-
 net/bridge/br_input.c        |    9 ++++++---
 net/bridge/br_netlink.c      |   10 +++++++++-
 net/bridge/br_private.h      |    7 +++++--
 net/bridge/br_sysfs_if.c     |    4 ++++
 9 files changed, 44 insertions(+), 16 deletions(-)

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] bridge: Add flag to control mac learning.
  2013-04-29 15:58 [PATCH net-next 0/2] Add some new flags to bridges Vlad Yasevich
@ 2013-04-29 15:58 ` Vlad Yasevich
  2013-04-29 16:11   ` Michael S. Tsirkin
  2013-04-29 15:58 ` [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood Vlad Yasevich
  1 sibling, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-29 15:58 UTC (permalink / raw)
  To: netdev; +Cc: bridge, mst, Vlad Yasevich

Allow user to control whether mac learning is enabled on the port.
By default, mac learning is enabled.  Disabling mac learning will
cause new dynamic FDB entries to not be created for a particular port.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_fdb.c          |    4 ++++
 net/bridge/br_if.c           |    2 +-
 net/bridge/br_netlink.c      |    6 +++++-
 net/bridge/br_private.h      |    1 +
 net/bridge/br_sysfs_if.c     |    2 ++
 6 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index e316354..80fad7f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -221,6 +221,7 @@ enum {
 	IFLA_BRPORT_GUARD,	/* bpdu guard              */
 	IFLA_BRPORT_PROTECT,	/* root port protection    */
 	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
+	IFLA_BRPORT_LEARNING,	/* mac learning */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index c581f12..9c4fe65 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -477,10 +477,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	} else {
 		spin_lock(&br->hash_lock);
 		if (likely(!fdb_find(head, addr, vid))) {
+			if (!(source->flags & BR_LEARNING))
+				goto unlock;
+
 			fdb = fdb_create(head, source, addr, vid);
 			if (fdb)
 				fdb_notify(br, fdb, RTM_NEWNEIGH);
 		}
+unlock:
 		/* else  we lose race and someone else inserts
 		 * it first, don't bother updating
 		 */
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f17fcb3..9751103 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -220,7 +220,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->path_cost = port_cost(dev);
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
-	p->flags = 0;
+	p->flags = BR_LEARNING;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 8e3abf5..ce902bf 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -30,6 +30,7 @@ static inline size_t br_port_info_size(void)
 		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
 		+ nla_total_size(1)	/* IFLA_BRPORT_FAST_LEAVE */
+		+ nla_total_size(1)	/* IFLA_BRPORT_LEARNING */
 		+ 0;
 }
 
@@ -56,7 +57,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
 	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)) ||
-	    nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE, !!(p->flags & BR_MULTICAST_FAST_LEAVE)))
+	    nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE, !!(p->flags & BR_MULTICAST_FAST_LEAVE)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)))
 		return -EMSGSIZE;
 
 	return 0;
@@ -281,6 +283,7 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_LEARNING]	= { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */
@@ -328,6 +331,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 	br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
 	br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
 	br_set_port_flag(p, tb, IFLA_BRPORT_PROTECT, BR_ROOT_BLOCK);
+	br_set_port_flag(p, tb, IFLA_BRPORT_LEARNING, BR_LEARNING);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 3cbf5be..67842b9 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -156,6 +156,7 @@ struct net_bridge_port
 #define BR_BPDU_GUARD           0x00000002
 #define BR_ROOT_BLOCK		0x00000004
 #define BR_MULTICAST_FAST_LEAVE	0x00000008
+#define BR_LEARNING		0x00000010
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index a1ef1b6..707f362 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -158,6 +158,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
+BRPORT_ATTR_FLAG(learning, BR_LEARNING);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -195,6 +196,7 @@ static const struct brport_attribute *brport_attrs[] = {
 	&brport_attr_hairpin_mode,
 	&brport_attr_bpdu_guard,
 	&brport_attr_root_block,
+	&brport_attr_learning,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 	&brport_attr_multicast_fast_leave,
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood.
  2013-04-29 15:58 [PATCH net-next 0/2] Add some new flags to bridges Vlad Yasevich
  2013-04-29 15:58 ` [PATCH net-next 1/2] bridge: Add flag to control mac learning Vlad Yasevich
@ 2013-04-29 15:58 ` Vlad Yasevich
  2013-04-29 16:12   ` Michael S. Tsirkin
  1 sibling, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-29 15:58 UTC (permalink / raw)
  To: netdev; +Cc: bridge, mst, Vlad Yasevich

Add a flag to control flood of unicast traffic.  By default, flood is
on and the bridge will flood unicast traffic if it doesn't know
the destination.  When the flag is turned off, unicast traffic
without an FDB will not be forwarded to the specified port.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_device.c       |    8 ++++----
 net/bridge/br_forward.c      |   14 +++++++++-----
 net/bridge/br_if.c           |    2 +-
 net/bridge/br_input.c        |    9 ++++++---
 net/bridge/br_netlink.c      |    6 +++++-
 net/bridge/br_private.h      |    6 ++++--
 net/bridge/br_sysfs_if.c     |    2 ++
 8 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 80fad7f..8d32021 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -222,6 +222,7 @@ enum {
 	IFLA_BRPORT_PROTECT,	/* root port protection    */
 	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
 	IFLA_BRPORT_LEARNING,	/* mac learning */
+	IFLA_BRPORT_UNICAST_FLOOD,	/* control flood of unicast traffic */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 9673128..014d690 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -55,10 +55,10 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	skb_pull(skb, ETH_HLEN);
 
 	if (is_broadcast_ether_addr(dest))
-		br_flood_deliver(br, skb);
+		br_flood_deliver(br, skb, false);
 	else if (is_multicast_ether_addr(dest)) {
 		if (unlikely(netpoll_tx_running(dev))) {
-			br_flood_deliver(br, skb);
+			br_flood_deliver(br, skb, false);
 			goto out;
 		}
 		if (br_multicast_rcv(br, NULL, skb)) {
@@ -70,11 +70,11 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb))
 			br_multicast_deliver(mdst, skb);
 		else
-			br_flood_deliver(br, skb);
+			br_flood_deliver(br, skb, false);
 	} else if ((dst = __br_fdb_get(br, dest, vid)) != NULL)
 		br_deliver(dst->dst, skb);
 	else
-		br_flood_deliver(br, skb);
+		br_flood_deliver(br, skb, true);
 
 out:
 	rcu_read_unlock();
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 092b20e..3ca17eb 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -174,7 +174,8 @@ out:
 static void br_flood(struct net_bridge *br, struct sk_buff *skb,
 		     struct sk_buff *skb0,
 		     void (*__packet_hook)(const struct net_bridge_port *p,
-					   struct sk_buff *skb))
+					   struct sk_buff *skb),
+		     bool unicast)
 {
 	struct net_bridge_port *p;
 	struct net_bridge_port *prev;
@@ -182,6 +183,9 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb,
 	prev = NULL;
 
 	list_for_each_entry_rcu(p, &br->port_list, list) {
+		/* Do not flood unicast traffic to ports that turn it off */
+		if (unicast && !(p->flags & BR_UNICAST_FLOOD))
+			continue;
 		prev = maybe_deliver(prev, p, skb, __packet_hook);
 		if (IS_ERR(prev))
 			goto out;
@@ -203,16 +207,16 @@ out:
 
 
 /* called with rcu_read_lock */
-void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb)
+void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb, bool unicast)
 {
-	br_flood(br, skb, NULL, __br_deliver);
+	br_flood(br, skb, NULL, __br_deliver, unicast);
 }
 
 /* called under bridge lock */
 void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,
-		      struct sk_buff *skb2)
+		      struct sk_buff *skb2, bool unicast)
 {
-	br_flood(br, skb, skb2, __br_forward);
+	br_flood(br, skb, skb2, __br_forward, unicast);
 }
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 9751103..3b15668 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -220,7 +220,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->path_cost = port_cost(dev);
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
-	p->flags = BR_LEARNING;
+	p->flags = BR_LEARNING | BR_UNICAST_FLOOD;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 828e2bc..6b6a49e 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -65,6 +65,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
 	struct net_bridge_fdb_entry *dst;
 	struct net_bridge_mdb_entry *mdst;
 	struct sk_buff *skb2;
+	bool unicast = true;
 	u16 vid = 0;
 
 	if (!p || p->state == BR_STATE_DISABLED)
@@ -94,9 +95,10 @@ int br_handle_frame_finish(struct sk_buff *skb)
 
 	dst = NULL;
 
-	if (is_broadcast_ether_addr(dest))
+	if (is_broadcast_ether_addr(dest)) {
 		skb2 = skb;
-	else if (is_multicast_ether_addr(dest)) {
+		unicast = false;
+	} else if (is_multicast_ether_addr(dest)) {
 		mdst = br_mdb_get(br, skb, vid);
 		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
 			if ((mdst && mdst->mglist) ||
@@ -109,6 +111,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
 		} else
 			skb2 = skb;
 
+		unicast = false;
 		br->dev->stats.multicast++;
 	} else if ((dst = __br_fdb_get(br, dest, vid)) &&
 			dst->is_local) {
@@ -122,7 +125,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
 			dst->used = jiffies;
 			br_forward(dst->dst, skb, skb2);
 		} else
-			br_flood_forward(br, skb, skb2);
+			br_flood_forward(br, skb, skb2, unicast);
 	}
 
 	if (skb2)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index ce902bf..d27599c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -31,6 +31,7 @@ static inline size_t br_port_info_size(void)
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
 		+ nla_total_size(1)	/* IFLA_BRPORT_FAST_LEAVE */
 		+ nla_total_size(1)	/* IFLA_BRPORT_LEARNING */
+		+ nla_total_size(1)	/* IFLA_BRPORT_UNICAST_FLOOD */
 		+ 0;
 }
 
@@ -58,7 +59,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE, !!(p->flags & BR_MULTICAST_FAST_LEAVE)) ||
-	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)))
+	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_UNICAST_FLOOD, !!(p->flags & BR_UNICAST_FLOOD)))
 		return -EMSGSIZE;
 
 	return 0;
@@ -284,6 +286,7 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_LEARNING]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_UNICAST_FLOOD] = { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */
@@ -332,6 +335,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 	br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
 	br_set_port_flag(p, tb, IFLA_BRPORT_PROTECT, BR_ROOT_BLOCK);
 	br_set_port_flag(p, tb, IFLA_BRPORT_LEARNING, BR_LEARNING);
+	br_set_port_flag(p, tb, IFLA_BRPORT_UNICAST_FLOOD, BR_UNICAST_FLOOD);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 67842b9..50b6f55 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -157,6 +157,7 @@ struct net_bridge_port
 #define BR_ROOT_BLOCK		0x00000004
 #define BR_MULTICAST_FAST_LEAVE	0x00000008
 #define BR_LEARNING		0x00000010
+#define BR_UNICAST_FLOOD	0x00000020
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
@@ -411,9 +412,10 @@ extern int br_dev_queue_push_xmit(struct sk_buff *skb);
 extern void br_forward(const struct net_bridge_port *to,
 		struct sk_buff *skb, struct sk_buff *skb0);
 extern int br_forward_finish(struct sk_buff *skb);
-extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb);
+extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb,
+			     bool unicast);
 extern void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,
-			     struct sk_buff *skb2);
+			     struct sk_buff *skb2, bool unicast);
 
 /* br_if.c */
 extern void br_port_carrier_check(struct net_bridge_port *p);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 707f362..ea6990f 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -159,6 +159,7 @@ BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
 BRPORT_ATTR_FLAG(learning, BR_LEARNING);
+BRPORT_ATTR_FLAG(unicast_flood, BR_UNICAST_FLOOD);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -197,6 +198,7 @@ static const struct brport_attribute *brport_attrs[] = {
 	&brport_attr_bpdu_guard,
 	&brport_attr_root_block,
 	&brport_attr_learning,
+	&brport_attr_unicast_flood,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 	&brport_attr_multicast_fast_leave,
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] bridge: Add flag to control mac learning.
  2013-04-29 15:58 ` [PATCH net-next 1/2] bridge: Add flag to control mac learning Vlad Yasevich
@ 2013-04-29 16:11   ` Michael S. Tsirkin
  2013-04-29 16:33     ` Vlad Yasevich
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-04-29 16:11 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, bridge

On Mon, Apr 29, 2013 at 11:58:21AM -0400, Vlad Yasevich wrote:
> Allow user to control whether mac learning is enabled on the port.
> By default, mac learning is enabled.  Disabling mac learning will
> cause new dynamic FDB entries to not be created for a particular port.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  include/uapi/linux/if_link.h |    1 +
>  net/bridge/br_fdb.c          |    4 ++++
>  net/bridge/br_if.c           |    2 +-
>  net/bridge/br_netlink.c      |    6 +++++-
>  net/bridge/br_private.h      |    1 +
>  net/bridge/br_sysfs_if.c     |    2 ++
>  6 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index e316354..80fad7f 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -221,6 +221,7 @@ enum {
>  	IFLA_BRPORT_GUARD,	/* bpdu guard              */
>  	IFLA_BRPORT_PROTECT,	/* root port protection    */
>  	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
> +	IFLA_BRPORT_LEARNING,	/* mac learning */
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index c581f12..9c4fe65 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -477,10 +477,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  	} else {
>  		spin_lock(&br->hash_lock);
>  		if (likely(!fdb_find(head, addr, vid))) {
> +			if (!(source->flags & BR_LEARNING))
> +				goto unlock;
> +
>  			fdb = fdb_create(head, source, addr, vid);
>  			if (fdb)
>  				fdb_notify(br, fdb, RTM_NEWNEIGH);
>  		}
> +unlock:
>  		/* else  we lose race and someone else inserts
>  		 * it first, don't bother updating
>  		 */

Wait a second, this will affect adding fdbs using RTM_NEWNEIGH, won't it?
And let's not bother with lookups if learning is off.
So I imagine we need something like the below instead
(in addition to the sysfs/netlink part of the patch which
look fine):

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index bab338e..30eaa6b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -448,8 +448,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
 	return ret;
 }
 
-void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr, u16 vid)
+static
+void __br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
 	struct net_bridge_fdb_entry *fdb;
@@ -490,6 +491,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	}
 }
 
+void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
+{
+	if (!(source->flags & BR_LEARNING))
+		return;
+	__br_fdb_update(br, source, addr, vid);
+}
+
 static int fdb_to_nud(const struct net_bridge_fdb_entry *fdb)
 {
 	if (fdb->is_local)
@@ -655,7 +664,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
 
 	if (ndm->ndm_flags & NTF_USE) {
 		rcu_read_lock();
-		br_fdb_update(p->br, p, addr, vid);
+		__br_fdb_update(p->br, p, addr, vid);
 		rcu_read_unlock();
 	} else {
 		spin_lock_bh(&p->br->hash_lock);
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood.
  2013-04-29 15:58 ` [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood Vlad Yasevich
@ 2013-04-29 16:12   ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-04-29 16:12 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, bridge

On Mon, Apr 29, 2013 at 11:58:22AM -0400, Vlad Yasevich wrote:
> Add a flag to control flood of unicast traffic.  By default, flood is
> on and the bridge will flood unicast traffic if it doesn't know
> the destination.  When the flag is turned off, unicast traffic
> without an FDB will not be forwarded to the specified port.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>

Looks sane

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  include/uapi/linux/if_link.h |    1 +
>  net/bridge/br_device.c       |    8 ++++----
>  net/bridge/br_forward.c      |   14 +++++++++-----
>  net/bridge/br_if.c           |    2 +-
>  net/bridge/br_input.c        |    9 ++++++---
>  net/bridge/br_netlink.c      |    6 +++++-
>  net/bridge/br_private.h      |    6 ++++--
>  net/bridge/br_sysfs_if.c     |    2 ++
>  8 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 80fad7f..8d32021 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -222,6 +222,7 @@ enum {
>  	IFLA_BRPORT_PROTECT,	/* root port protection    */
>  	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
>  	IFLA_BRPORT_LEARNING,	/* mac learning */
> +	IFLA_BRPORT_UNICAST_FLOOD,	/* control flood of unicast traffic */
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index 9673128..014d690 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -55,10 +55,10 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  	skb_pull(skb, ETH_HLEN);
>  
>  	if (is_broadcast_ether_addr(dest))
> -		br_flood_deliver(br, skb);
> +		br_flood_deliver(br, skb, false);
>  	else if (is_multicast_ether_addr(dest)) {
>  		if (unlikely(netpoll_tx_running(dev))) {
> -			br_flood_deliver(br, skb);
> +			br_flood_deliver(br, skb, false);
>  			goto out;
>  		}
>  		if (br_multicast_rcv(br, NULL, skb)) {
> @@ -70,11 +70,11 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb))
>  			br_multicast_deliver(mdst, skb);
>  		else
> -			br_flood_deliver(br, skb);
> +			br_flood_deliver(br, skb, false);
>  	} else if ((dst = __br_fdb_get(br, dest, vid)) != NULL)
>  		br_deliver(dst->dst, skb);
>  	else
> -		br_flood_deliver(br, skb);
> +		br_flood_deliver(br, skb, true);
>  
>  out:
>  	rcu_read_unlock();
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 092b20e..3ca17eb 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -174,7 +174,8 @@ out:
>  static void br_flood(struct net_bridge *br, struct sk_buff *skb,
>  		     struct sk_buff *skb0,
>  		     void (*__packet_hook)(const struct net_bridge_port *p,
> -					   struct sk_buff *skb))
> +					   struct sk_buff *skb),
> +		     bool unicast)
>  {
>  	struct net_bridge_port *p;
>  	struct net_bridge_port *prev;
> @@ -182,6 +183,9 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb,
>  	prev = NULL;
>  
>  	list_for_each_entry_rcu(p, &br->port_list, list) {
> +		/* Do not flood unicast traffic to ports that turn it off */
> +		if (unicast && !(p->flags & BR_UNICAST_FLOOD))
> +			continue;
>  		prev = maybe_deliver(prev, p, skb, __packet_hook);
>  		if (IS_ERR(prev))
>  			goto out;
> @@ -203,16 +207,16 @@ out:
>  
>  
>  /* called with rcu_read_lock */
> -void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb)
> +void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb, bool unicast)
>  {
> -	br_flood(br, skb, NULL, __br_deliver);
> +	br_flood(br, skb, NULL, __br_deliver, unicast);
>  }
>  
>  /* called under bridge lock */
>  void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,
> -		      struct sk_buff *skb2)
> +		      struct sk_buff *skb2, bool unicast)
>  {
> -	br_flood(br, skb, skb2, __br_forward);
> +	br_flood(br, skb, skb2, __br_forward, unicast);
>  }
>  
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 9751103..3b15668 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -220,7 +220,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>  	p->path_cost = port_cost(dev);
>  	p->priority = 0x8000 >> BR_PORT_BITS;
>  	p->port_no = index;
> -	p->flags = BR_LEARNING;
> +	p->flags = BR_LEARNING | BR_UNICAST_FLOOD;
>  	br_init_port(p);
>  	p->state = BR_STATE_DISABLED;
>  	br_stp_port_timer_init(p);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 828e2bc..6b6a49e 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -65,6 +65,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
>  	struct net_bridge_fdb_entry *dst;
>  	struct net_bridge_mdb_entry *mdst;
>  	struct sk_buff *skb2;
> +	bool unicast = true;
>  	u16 vid = 0;
>  
>  	if (!p || p->state == BR_STATE_DISABLED)
> @@ -94,9 +95,10 @@ int br_handle_frame_finish(struct sk_buff *skb)
>  
>  	dst = NULL;
>  
> -	if (is_broadcast_ether_addr(dest))
> +	if (is_broadcast_ether_addr(dest)) {
>  		skb2 = skb;
> -	else if (is_multicast_ether_addr(dest)) {
> +		unicast = false;
> +	} else if (is_multicast_ether_addr(dest)) {
>  		mdst = br_mdb_get(br, skb, vid);
>  		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
>  			if ((mdst && mdst->mglist) ||
> @@ -109,6 +111,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
>  		} else
>  			skb2 = skb;
>  
> +		unicast = false;
>  		br->dev->stats.multicast++;
>  	} else if ((dst = __br_fdb_get(br, dest, vid)) &&
>  			dst->is_local) {
> @@ -122,7 +125,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
>  			dst->used = jiffies;
>  			br_forward(dst->dst, skb, skb2);
>  		} else
> -			br_flood_forward(br, skb, skb2);
> +			br_flood_forward(br, skb, skb2, unicast);
>  	}
>  
>  	if (skb2)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index ce902bf..d27599c 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -31,6 +31,7 @@ static inline size_t br_port_info_size(void)
>  		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_FAST_LEAVE */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_LEARNING */
> +		+ nla_total_size(1)	/* IFLA_BRPORT_UNICAST_FLOOD */
>  		+ 0;
>  }
>  
> @@ -58,7 +59,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>  	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE, !!(p->flags & BR_MULTICAST_FAST_LEAVE)) ||
> -	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)))
> +	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)) ||
> +	    nla_put_u8(skb, IFLA_BRPORT_UNICAST_FLOOD, !!(p->flags & BR_UNICAST_FLOOD)))
>  		return -EMSGSIZE;
>  
>  	return 0;
> @@ -284,6 +286,7 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
>  	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
>  	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
>  	[IFLA_BRPORT_LEARNING]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_UNICAST_FLOOD] = { .type = NLA_U8 },
>  };
>  
>  /* Change the state of the port and notify spanning tree */
> @@ -332,6 +335,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
>  	br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
>  	br_set_port_flag(p, tb, IFLA_BRPORT_PROTECT, BR_ROOT_BLOCK);
>  	br_set_port_flag(p, tb, IFLA_BRPORT_LEARNING, BR_LEARNING);
> +	br_set_port_flag(p, tb, IFLA_BRPORT_UNICAST_FLOOD, BR_UNICAST_FLOOD);
>  
>  	if (tb[IFLA_BRPORT_COST]) {
>  		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 67842b9..50b6f55 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -157,6 +157,7 @@ struct net_bridge_port
>  #define BR_ROOT_BLOCK		0x00000004
>  #define BR_MULTICAST_FAST_LEAVE	0x00000008
>  #define BR_LEARNING		0x00000010
> +#define BR_UNICAST_FLOOD	0x00000020
>  
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>  	u32				multicast_startup_queries_sent;
> @@ -411,9 +412,10 @@ extern int br_dev_queue_push_xmit(struct sk_buff *skb);
>  extern void br_forward(const struct net_bridge_port *to,
>  		struct sk_buff *skb, struct sk_buff *skb0);
>  extern int br_forward_finish(struct sk_buff *skb);
> -extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb);
> +extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb,
> +			     bool unicast);
>  extern void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,
> -			     struct sk_buff *skb2);
> +			     struct sk_buff *skb2, bool unicast);
>  
>  /* br_if.c */
>  extern void br_port_carrier_check(struct net_bridge_port *p);
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index 707f362..ea6990f 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -159,6 +159,7 @@ BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
>  BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
>  BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
>  BRPORT_ATTR_FLAG(learning, BR_LEARNING);
> +BRPORT_ATTR_FLAG(unicast_flood, BR_UNICAST_FLOOD);
>  
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>  static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
> @@ -197,6 +198,7 @@ static const struct brport_attribute *brport_attrs[] = {
>  	&brport_attr_bpdu_guard,
>  	&brport_attr_root_block,
>  	&brport_attr_learning,
> +	&brport_attr_unicast_flood,
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>  	&brport_attr_multicast_router,
>  	&brport_attr_multicast_fast_leave,
> -- 
> 1.7.7.6

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] bridge: Add flag to control mac learning.
  2013-04-29 16:11   ` Michael S. Tsirkin
@ 2013-04-29 16:33     ` Vlad Yasevich
  0 siblings, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-29 16:33 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, bridge

On 04/29/2013 12:11 PM, Michael S. Tsirkin wrote:
> On Mon, Apr 29, 2013 at 11:58:21AM -0400, Vlad Yasevich wrote:
>> Allow user to control whether mac learning is enabled on the port.
>> By default, mac learning is enabled.  Disabling mac learning will
>> cause new dynamic FDB entries to not be created for a particular port.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   include/uapi/linux/if_link.h |    1 +
>>   net/bridge/br_fdb.c          |    4 ++++
>>   net/bridge/br_if.c           |    2 +-
>>   net/bridge/br_netlink.c      |    6 +++++-
>>   net/bridge/br_private.h      |    1 +
>>   net/bridge/br_sysfs_if.c     |    2 ++
>>   6 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index e316354..80fad7f 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -221,6 +221,7 @@ enum {
>>   	IFLA_BRPORT_GUARD,	/* bpdu guard              */
>>   	IFLA_BRPORT_PROTECT,	/* root port protection    */
>>   	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
>> +	IFLA_BRPORT_LEARNING,	/* mac learning */
>>   	__IFLA_BRPORT_MAX
>>   };
>>   #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index c581f12..9c4fe65 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -477,10 +477,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>   	} else {
>>   		spin_lock(&br->hash_lock);
>>   		if (likely(!fdb_find(head, addr, vid))) {
>> +			if (!(source->flags & BR_LEARNING))
>> +				goto unlock;
>> +
>>   			fdb = fdb_create(head, source, addr, vid);
>>   			if (fdb)
>>   				fdb_notify(br, fdb, RTM_NEWNEIGH);
>>   		}
>> +unlock:
>>   		/* else  we lose race and someone else inserts
>>   		 * it first, don't bother updating
>>   		 */
>
> Wait a second, this will affect adding fdbs using RTM_NEWNEIGH, won't it?

Hm..  Yes, I missed this.  But this has got me thinking of whether it is 
correct to allow 'creation' of an entry if NTF_USE is set.

Consider, that the current behavior is rather broken in that
if someone sets  NLM_F_CREATE|NLM_F_EXCL and NTF_USE, we will update
the FDB (which is technically wrong since we ignore the exclusivity).
Also, if someone doesn't specify NLM_F_CREATE, we create anyway.

I think that may be I'll fix the above inconsistencies first.

-vlad

> And let's not bother with lookups if learning is off.
> So I imagine we need something like the below instead
> (in addition to the sysfs/netlink part of the patch which
> look fine):
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>

> ---
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index bab338e..30eaa6b 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -448,8 +448,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>   	return ret;
>   }
>
> -void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> -		   const unsigned char *addr, u16 vid)
> +static
> +void __br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> +		     const unsigned char *addr, u16 vid)
>   {
>   	struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
>   	struct net_bridge_fdb_entry *fdb;
> @@ -490,6 +491,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>   	}
>   }
>
> +void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> +		     const unsigned char *addr, u16 vid)
> +{
> +	if (!(source->flags & BR_LEARNING))
> +		return;
> +	__br_fdb_update(br, source, addr, vid);
> +}
> +
>   static int fdb_to_nud(const struct net_bridge_fdb_entry *fdb)
>   {
>   	if (fdb->is_local)
> @@ -655,7 +664,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
>
>   	if (ndm->ndm_flags & NTF_USE) {
>   		rcu_read_lock();
> -		br_fdb_update(p->br, p, addr, vid);
> +		__br_fdb_update(p->br, p, addr, vid);
>   		rcu_read_unlock();
>   	} else {
>   		spin_lock_bh(&p->br->hash_lock);
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-29 16:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-29 15:58 [PATCH net-next 0/2] Add some new flags to bridges Vlad Yasevich
2013-04-29 15:58 ` [PATCH net-next 1/2] bridge: Add flag to control mac learning Vlad Yasevich
2013-04-29 16:11   ` Michael S. Tsirkin
2013-04-29 16:33     ` Vlad Yasevich
2013-04-29 15:58 ` [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood Vlad Yasevich
2013-04-29 16:12   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).