Menu
▾
▴
Re: [tipc-discussion] [net-next] tipc: support multicast/replicast configurable for bearer
|
From: Jon M. <jon...@er...> - 2019-01-31 00:33:20
|
Hi Hoang,
I tried this, but found it a little limiting in comparison to what I had in mind. I suggest we make another iteration on this.
First of all, we shouldn't any bearers or links at all, -only the tipc_bc_base structure.
We should add three options to the 'tipc link set broadcast' command:
- BROADCAST
- REPLICAST
- SELECTABLE
These values have a different meaning from what you have implemented:
- BROADCAST forces all multicast traffic to be transmitted via broadcast only, irrespective of cluster size and number of destinations.
You achieve this by setting a new field tipc_bc_base::force_bcast to true and the corresponding force_rcast to false. This setting must however be rejected if the corresponding tipc_bc_base::bc_supported is already is false, i.e., one of the bearers doesn't support broadcast. This option is useful when we e.g., want to test the broadcast implementation in a small cluster.
-REPLICAST forces all multicast traffic to be transmitted via replicast only, irrespective of cluster size and number of destinations.
You achieve this by setting a new field tipc_bc_base::force_rcast to true and the corresponding force_bcast to false. This setting must however be rejected if the corresponding tipc_bc_base::rc_supported is already is false, i.e., one of the destination nodes (old nodes) doesn't support replicast.This option is the one that overrides VXLAN's emulated broadcast setting.
- SELECTABLE means that the system can switch between broadcast and replicast depending on cluster size and destination number. Force_bcast and force_rcast are both set to false. This setting must be rejected if one of the fields bc_supported or rc_supported is false. When given this option, an extra option must also be available: 'ratio' which determines the selection threshold. Currently, the rc_ratio is hard coded to 25%, which is probably too high. SELECTABLE should be the default broadcast value (when possible) and the default for 'ratio' should be 10 (meaning 10 %).
When we do 'get', we do get the current value, including the ratio when it is relevant (given with the '%' character, to help the user to understand what it is) when the broadcast value is SELECTABLE.
The 'link' option is redundant and confusing. This property can only be set on the broadcast link, and should only be listed as a property of that link, not the others.
Also, when 'tipc link set broadcast <return>' is typed the help text should print out the three options, so the user gets a clue about what to set. Later, we need to update the man pages and bash command completion script with this.
I think this, in combination with your other rcast/bcast patch should give us what we need in all types of environments for the future.
Regards
///jon
> -----Original Message-----
> From: Hoang Le <hoa...@de...>
> Sent: 28-Jan-19 21:30
> To: Jon Maloy <jon...@er...>; ma...@do...;
> yin...@wi...; tip...@li...
> Subject: [net-next] tipc: support multicast/replicast configurable for bearer
>
> When L2 interface (e.g VXLAN) pseudo support broadcast, we should make it
> possible for the user to disable this manually and switch to replicast.
>
> We should preferably find a way to sense this in the code, but if we cannot
> this feature also useful to manual configuration.
>
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
> include/uapi/linux/tipc_netlink.h | 3 ++-
> net/tipc/bcast.c | 24 ++++++++++++++++++++++--
> net/tipc/bcast.h | 2 ++
> net/tipc/link.c | 6 ++++++
> 4 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/tipc_netlink.h
> b/include/uapi/linux/tipc_netlink.h
> index 0ebe02ef1a86..a4a765f69352 100644
> --- a/include/uapi/linux/tipc_netlink.h
> +++ b/include/uapi/linux/tipc_netlink.h
> @@ -273,7 +273,7 @@ enum {
> TIPC_NLA_SOCK_STAT_MAX = __TIPC_NLA_SOCK_STAT_MAX - 1 };
>
> -/* Nest, link propreties. Valid for link, media and bearer */
> +/* Nest, link properties. Valid for link, media and bearer */
> enum {
> TIPC_NLA_PROP_UNSPEC,
>
> @@ -281,6 +281,7 @@ enum {
> TIPC_NLA_PROP_TOL, /* u32 */
> TIPC_NLA_PROP_WIN, /* u32 */
> TIPC_NLA_PROP_MTU, /* u32 */
> + TIPC_NLA_PROP_BCAST, /* u8 */
>
> __TIPC_NLA_PROP_MAX,
> TIPC_NLA_PROP_MAX = __TIPC_NLA_PROP_MAX - 1 diff --git
> a/net/tipc/bcast.c b/net/tipc/bcast.c index d8026543bf4c..44acd6957b65
> 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -88,6 +88,16 @@ void tipc_bcast_disable_rcast(struct net *net)
> tipc_bc_base(net)->rcast_support = false; }
>
> +bool tipc_bcast_get_bcast(struct net *net) {
> + return tipc_bc_base(net)->bcast_support; }
> +
> +void tipc_bcast_set_bcast(struct net *net, bool sup) {
> + tipc_bc_base(net)->bcast_support = sup; }
> +
> static void tipc_bcbase_calc_bc_threshold(struct net *net) {
> struct tipc_bc_base *bb = tipc_bc_base(net); @@ -106,7 +116,6 @@
> static void tipc_bcbase_select_primary(struct net *net)
> int i, mtu, prim;
>
> bb->primary_bearer = INVALID_BEARER_ID;
> - bb->bcast_support = true;
>
> if (!all_dests)
> return;
> @@ -130,7 +139,7 @@ static void tipc_bcbase_select_primary(struct net
> *net)
> }
> prim = bb->primary_bearer;
> if (prim != INVALID_BEARER_ID)
> - bb->bcast_support = tipc_bearer_bcast_support(net, prim);
> + bb->bcast_support &= tipc_bearer_bcast_support(net,
> prim);
> }
>
> void tipc_bcast_inc_bearer_dst_cnt(struct net *net, int bearer_id) @@ -
> 498,6 +507,16 @@ int tipc_nl_bc_link_set(struct net *net, struct nlattr
> *attrs[])
> if (err)
> return err;
>
> + if (props[TIPC_NLA_PROP_BCAST]) {
> + tipc_bcast_lock(net);
> + if (nla_get_u8(props[TIPC_NLA_PROP_BCAST]) & 0x1)
> + tipc_bcast_set_bcast(net, true);
> + else
> + tipc_bcast_set_bcast(net, false);
> + tipc_bcast_unlock(net);
> + return 0;
> + }
> +
> if (!props[TIPC_NLA_PROP_WIN])
> return -EOPNOTSUPP;
>
> @@ -531,6 +550,7 @@ int tipc_bcast_init(struct net *net)
> tn->bcl = l;
> bb->rc_ratio = 25;
> bb->rcast_support = true;
> + bb->bcast_support = true;
> return 0;
> enomem:
> kfree(bb);
> diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index
> 751530ab0c49..8880dc595c0b 100644
> --- a/net/tipc/bcast.h
> +++ b/net/tipc/bcast.h
> @@ -91,6 +91,8 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link
> *l, int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); int
> tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); int
> tipc_bclink_reset_stats(struct net *net);
> +void tipc_bcast_set_bcast(struct net *net, bool sup); bool
> +tipc_bcast_get_bcast(struct net *net);
>
> static inline void tipc_bcast_lock(struct net *net) { diff --git a/net/tipc/link.c
> b/net/tipc/link.c index ac306d17f8ad..0ac0943b1b00 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -2103,6 +2103,9 @@ int __tipc_nl_add_link(struct net *net, struct
> tipc_nl_msg *msg,
> goto prop_msg_full;
> if (nla_put_u32(msg->skb, TIPC_NLA_PROP_PRIO, link->priority))
> goto prop_msg_full;
> + if (nla_put_u8(msg->skb, TIPC_NLA_PROP_BCAST,
> + tipc_bearer_bcast_support(net, link->bearer_id)))
> + goto prop_msg_full;
> nla_nest_end(msg->skb, prop);
>
> err = __tipc_nl_add_stats(msg->skb, &link->stats); @@ -2183,6
> +2186,7 @@ int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg
> *msg)
> struct nlattr *prop;
> struct tipc_net *tn = net_generic(net, tipc_net_id);
> struct tipc_link *bcl = tn->bcl;
> + bool bcast = tipc_bcast_get_bcast(net);
>
> if (!bcl)
> return 0;
> @@ -2218,6 +2222,8 @@ int tipc_nl_add_bc_link(struct net *net, struct
> tipc_nl_msg *msg)
> goto attr_msg_full;
> if (nla_put_u32(msg->skb, TIPC_NLA_PROP_WIN, bcl->window))
> goto prop_msg_full;
> + if (nla_put_u8(msg->skb, TIPC_NLA_PROP_BCAST, bcast))
> + goto prop_msg_full;
> nla_nest_end(msg->skb, prop);
>
> err = __tipc_nl_add_bc_link_stat(msg->skb, &bcl->stats);
> --
> 2.17.1
|
