[patch net-next v2 0/3] team: couple of patches

[patch net-next v2 0/3] team: couple of patches

[Jiri Pirko]

Do long time in-todo-queue convert of port list netlik messages to multipart
ones. That was needed in order to reorder calls in team_port_del(). That
was needed for correct event send of active port zeroing.

Jiri Pirko (3):
  team: handle sending port list in the same way option list is sent
  team: move netlink event notifiers after team_port_leave()
  team: ab: set active port option as changed when port is leaving

 drivers/net/team/team.c                   | 187 ++++++++++++++++--------------
 drivers/net/team/team_mode_activebackup.c |  13 ++-
 2 files changed, 111 insertions(+), 89 deletions(-)

-- 
1.8.1

[patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Jiri Pirko]

Essentially do the same thing with port list as with option list.
Multipart netlink message.
Side effect is that port event message can send port which is not longer
in team->port_list.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team.c | 177 +++++++++++++++++++++++++-----------------------
 1 file changed, 93 insertions(+), 84 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 70d5d6b..738f744 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1965,30 +1965,6 @@ static void team_nl_team_put(struct team *team)
 	dev_put(team->dev);
 }
 
-static int team_nl_send_generic(struct genl_info *info, struct team *team,
-				int (*fill_func)(struct sk_buff *skb,
-						 struct genl_info *info,
-						 int flags, struct team *team))
-{
-	struct sk_buff *skb;
-	int err;
-
-	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!skb)
-		return -ENOMEM;
-
-	err = fill_func(skb, info, NLM_F_ACK, team);
-	if (err < 0)
-		goto err_fill;
-
-	err = genlmsg_unicast(genl_info_net(info), skb, info->snd_portid);
-	return err;
-
-err_fill:
-	nlmsg_free(skb);
-	return err;
-}
-
 typedef int team_nl_send_func_t(struct sk_buff *skb,
 				struct team *team, u32 portid);
 
@@ -2333,16 +2309,57 @@ team_put:
 	return err;
 }
 
-static int team_nl_fill_port_list_get(struct sk_buff *skb,
-				      u32 portid, u32 seq, int flags,
-				      struct team *team,
-				      bool fillall)
+static int team_nl_fill_one_port_get(struct sk_buff *skb,
+				     struct team_port *port)
+{
+	struct nlattr *port_item;
+
+	port_item = nla_nest_start(skb, TEAM_ATTR_ITEM_PORT);
+	if (!port_item)
+		goto nest_cancel;
+	if (nla_put_u32(skb, TEAM_ATTR_PORT_IFINDEX, port->dev->ifindex))
+		goto nest_cancel;
+	if (port->changed) {
+		if (nla_put_flag(skb, TEAM_ATTR_PORT_CHANGED))
+			goto nest_cancel;
+		port->changed = false;
+	}
+	if ((port->removed &&
+	     nla_put_flag(skb, TEAM_ATTR_PORT_REMOVED)) ||
+	    (port->state.linkup &&
+	     nla_put_flag(skb, TEAM_ATTR_PORT_LINKUP)) ||
+	    nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->state.speed) ||
+	    nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->state.duplex))
+		goto nest_cancel;
+	nla_nest_end(skb, port_item);
+	return 0;
+
+nest_cancel:
+	nla_nest_cancel(skb, port_item);
+	return -EMSGSIZE;
+}
+
+static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
+				      int flags, team_nl_send_func_t *send_func,
+				      struct team_port *one_port)
 {
 	struct nlattr *port_list;
+	struct nlmsghdr *nlh;
 	void *hdr;
 	struct team_port *port;
+	int err;
+	struct sk_buff *skb = NULL;
+	bool incomplete;
+	int i;
 
-	hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags,
+	port = list_first_entry(&team->port_list, struct team_port, list);
+
+start_again:
+	err = __send_and_alloc_skb(&skb, team, portid, send_func);
+	if (err)
+		return err;
+
+	hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
 			  TEAM_CMD_PORT_LIST_GET);
 	if (!hdr)
 		return -EMSGSIZE;
@@ -2353,47 +2370,54 @@ static int team_nl_fill_port_list_get(struct sk_buff *skb,
 	if (!port_list)
 		goto nla_put_failure;
 
-	list_for_each_entry(port, &team->port_list, list) {
-		struct nlattr *port_item;
+	i = 0;
+	incomplete = false;
 
-		/* Include only changed ports if fill all mode is not on */
-		if (!fillall && !port->changed)
-			continue;
-		port_item = nla_nest_start(skb, TEAM_ATTR_ITEM_PORT);
-		if (!port_item)
-			goto nla_put_failure;
-		if (nla_put_u32(skb, TEAM_ATTR_PORT_IFINDEX, port->dev->ifindex))
-			goto nla_put_failure;
-		if (port->changed) {
-			if (nla_put_flag(skb, TEAM_ATTR_PORT_CHANGED))
-				goto nla_put_failure;
-			port->changed = false;
+	/* If one port is selected, called wants to send port list containing
+	 * only this port. Otherwise go through all listed ports and send all
+	 */
+	if (one_port) {
+		err = team_nl_fill_one_port_get(skb, one_port);
+		if (err)
+			goto errout;
+	} else {
+		list_for_each_entry(port, &team->port_list, list) {
+			err = team_nl_fill_one_port_get(skb, port);
+			if (err) {
+				if (err == -EMSGSIZE) {
+					if (!i)
+						goto errout;
+					incomplete = true;
+					break;
+				}
+				goto errout;
+			}
+			i++;
 		}
-		if ((port->removed &&
-		     nla_put_flag(skb, TEAM_ATTR_PORT_REMOVED)) ||
-		    (port->state.linkup &&
-		     nla_put_flag(skb, TEAM_ATTR_PORT_LINKUP)) ||
-		    nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->state.speed) ||
-		    nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->state.duplex))
-			goto nla_put_failure;
-		nla_nest_end(skb, port_item);
 	}
 
 	nla_nest_end(skb, port_list);
-	return genlmsg_end(skb, hdr);
+	genlmsg_end(skb, hdr);
+	if (incomplete)
+		goto start_again;
+
+send_done:
+	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
+	if (!nlh) {
+		err = __send_and_alloc_skb(&skb, team, portid, send_func);
+		if (err)
+			goto errout;
+		goto send_done;
+	}
+
+	return send_func(skb, team, portid);
 
 nla_put_failure:
+	err = -EMSGSIZE;
+errout:
 	genlmsg_cancel(skb, hdr);
-	return -EMSGSIZE;
-}
-
-static int team_nl_fill_port_list_get_all(struct sk_buff *skb,
-					  struct genl_info *info, int flags,
-					  struct team *team)
-{
-	return team_nl_fill_port_list_get(skb, info->snd_portid,
-					  info->snd_seq, NLM_F_ACK,
-					  team, true);
+	nlmsg_free(skb);
+	return err;
 }
 
 static int team_nl_cmd_port_list_get(struct sk_buff *skb,
@@ -2406,7 +2430,8 @@ static int team_nl_cmd_port_list_get(struct sk_buff *skb,
 	if (!team)
 		return -EINVAL;
 
-	err = team_nl_send_generic(info, team, team_nl_fill_port_list_get_all);
+	err = team_nl_send_port_list_get(team, info->snd_portid, info->snd_seq,
+					 NLM_F_ACK, team_nl_send_unicast, NULL);
 
 	team_nl_team_put(team);
 
@@ -2457,27 +2482,11 @@ static int team_nl_send_event_options_get(struct team *team,
 					sel_opt_inst_list);
 }
 
-static int team_nl_send_event_port_list_get(struct team *team)
+static int team_nl_send_event_port_get(struct team *team,
+				       struct team_port *port)
 {
-	struct sk_buff *skb;
-	int err;
-	struct net *net = dev_net(team->dev);
-
-	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!skb)
-		return -ENOMEM;
-
-	err = team_nl_fill_port_list_get(skb, 0, 0, 0, team, false);
-	if (err < 0)
-		goto err_fill;
-
-	err = genlmsg_multicast_netns(net, skb, 0, team_change_event_mcgrp.id,
-				      GFP_KERNEL);
-	return err;
-
-err_fill:
-	nlmsg_free(skb);
-	return err;
+	return team_nl_send_port_list_get(team, 0, 0, 0, team_nl_send_multicast,
+					  port);
 }
 
 static int team_nl_init(void)
@@ -2550,7 +2559,7 @@ static void __team_port_change_send(struct team_port *port, bool linkup)
 	port->state.duplex = 0;
 
 send_event:
-	err = team_nl_send_event_port_list_get(port->team);
+	err = team_nl_send_event_port_get(port->team, port);
 	if (err && err != -ESRCH)
 		netdev_warn(port->team->dev, "Failed to send port change of device %s via netlink (err %d)\n",
 			    port->dev->name, err);
-- 
1.8.1

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Pablo Neira Ayuso]

Hi Jiri,

On Fri, Feb 01, 2013 at 07:17:24PM +0100, Jiri Pirko wrote:
> Essentially do the same thing with port list as with option list.
> Multipart netlink message.
> Side effect is that port event message can send port which is not longer
> in team->port_list.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  drivers/net/team/team.c | 177 +++++++++++++++++++++++++-----------------------
>  1 file changed, 93 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 70d5d6b..738f744 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -1965,30 +1965,6 @@ static void team_nl_team_put(struct team *team)
>  	dev_put(team->dev);
>  }
>  
> -static int team_nl_send_generic(struct genl_info *info, struct team *team,
> -				int (*fill_func)(struct sk_buff *skb,
> -						 struct genl_info *info,
> -						 int flags, struct team *team))
> -{
> -	struct sk_buff *skb;
> -	int err;
> -
> -	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> -	if (!skb)
> -		return -ENOMEM;
> -
> -	err = fill_func(skb, info, NLM_F_ACK, team);
> -	if (err < 0)
> -		goto err_fill;
> -
> -	err = genlmsg_unicast(genl_info_net(info), skb, info->snd_portid);
> -	return err;
> -
> -err_fill:
> -	nlmsg_free(skb);
> -	return err;
> -}
> -
>  typedef int team_nl_send_func_t(struct sk_buff *skb,
>  				struct team *team, u32 portid);
>  
> @@ -2333,16 +2309,57 @@ team_put:
>  	return err;
>  }
>  
> -static int team_nl_fill_port_list_get(struct sk_buff *skb,
> -				      u32 portid, u32 seq, int flags,
> -				      struct team *team,
> -				      bool fillall)
> +static int team_nl_fill_one_port_get(struct sk_buff *skb,
> +				     struct team_port *port)
> +{
> +	struct nlattr *port_item;
> +
> +	port_item = nla_nest_start(skb, TEAM_ATTR_ITEM_PORT);
> +	if (!port_item)
> +		goto nest_cancel;
> +	if (nla_put_u32(skb, TEAM_ATTR_PORT_IFINDEX, port->dev->ifindex))
> +		goto nest_cancel;
> +	if (port->changed) {
> +		if (nla_put_flag(skb, TEAM_ATTR_PORT_CHANGED))
> +			goto nest_cancel;
> +		port->changed = false;
> +	}
> +	if ((port->removed &&
> +	     nla_put_flag(skb, TEAM_ATTR_PORT_REMOVED)) ||
> +	    (port->state.linkup &&
> +	     nla_put_flag(skb, TEAM_ATTR_PORT_LINKUP)) ||
> +	    nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->state.speed) ||
> +	    nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->state.duplex))
> +		goto nest_cancel;
> +	nla_nest_end(skb, port_item);
> +	return 0;
> +
> +nest_cancel:
> +	nla_nest_cancel(skb, port_item);
> +	return -EMSGSIZE;
> +}
> +
> +static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
> +				      int flags, team_nl_send_func_t *send_func,
> +				      struct team_port *one_port)
>  {
>  	struct nlattr *port_list;
> +	struct nlmsghdr *nlh;
>  	void *hdr;
>  	struct team_port *port;
> +	int err;
> +	struct sk_buff *skb = NULL;
> +	bool incomplete;
> +	int i;
>  
> -	hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags,
> +	port = list_first_entry(&team->port_list, struct team_port, list);
> +
> +start_again:
> +	err = __send_and_alloc_skb(&skb, team, portid, send_func);
> +	if (err)
> +		return err;
> +
> +	hdr = genlmsg_put(skb, portid, seq, &team_nl_family, flags | NLM_F_MULTI,
>  			  TEAM_CMD_PORT_LIST_GET);
>  	if (!hdr)
>  		return -EMSGSIZE;
> @@ -2353,47 +2370,54 @@ static int team_nl_fill_port_list_get(struct sk_buff *skb,
>  	if (!port_list)
>  		goto nla_put_failure;
>  
> -	list_for_each_entry(port, &team->port_list, list) {
> -		struct nlattr *port_item;
> +	i = 0;
> +	incomplete = false;
>  
> -		/* Include only changed ports if fill all mode is not on */
> -		if (!fillall && !port->changed)
> -			continue;
> -		port_item = nla_nest_start(skb, TEAM_ATTR_ITEM_PORT);
> -		if (!port_item)
> -			goto nla_put_failure;
> -		if (nla_put_u32(skb, TEAM_ATTR_PORT_IFINDEX, port->dev->ifindex))
> -			goto nla_put_failure;
> -		if (port->changed) {
> -			if (nla_put_flag(skb, TEAM_ATTR_PORT_CHANGED))
> -				goto nla_put_failure;
> -			port->changed = false;
> +	/* If one port is selected, called wants to send port list containing
> +	 * only this port. Otherwise go through all listed ports and send all
> +	 */
> +	if (one_port) {
> +		err = team_nl_fill_one_port_get(skb, one_port);
> +		if (err)
> +			goto errout;
> +	} else {
> +		list_for_each_entry(port, &team->port_list, list) {
> +			err = team_nl_fill_one_port_get(skb, port);
> +			if (err) {
> +				if (err == -EMSGSIZE) {
> +					if (!i)
> +						goto errout;
> +					incomplete = true;
> +					break;
> +				}
> +				goto errout;
> +			}
> +			i++;
>  		}
> -		if ((port->removed &&
> -		     nla_put_flag(skb, TEAM_ATTR_PORT_REMOVED)) ||
> -		    (port->state.linkup &&
> -		     nla_put_flag(skb, TEAM_ATTR_PORT_LINKUP)) ||
> -		    nla_put_u32(skb, TEAM_ATTR_PORT_SPEED, port->state.speed) ||
> -		    nla_put_u8(skb, TEAM_ATTR_PORT_DUPLEX, port->state.duplex))
> -			goto nla_put_failure;
> -		nla_nest_end(skb, port_item);
>  	}
>  
>  	nla_nest_end(skb, port_list);
> -	return genlmsg_end(skb, hdr);
> +	genlmsg_end(skb, hdr);
> +	if (incomplete)
> +		goto start_again;
> +
> +send_done:
> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
> +	if (!nlh) {
> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
> +		if (err)
> +			goto errout;
> +		goto send_done;
> +	}

I'd suggest to use netlink_dump_start for this, so you don't need to
manually create the NLMSG_DONE message.

> +
> +	return send_func(skb, team, portid);
>  
>  nla_put_failure:
> +	err = -EMSGSIZE;
> +errout:
>  	genlmsg_cancel(skb, hdr);
> -	return -EMSGSIZE;
> -}
> -
> -static int team_nl_fill_port_list_get_all(struct sk_buff *skb,
> -					  struct genl_info *info, int flags,
> -					  struct team *team)
> -{
> -	return team_nl_fill_port_list_get(skb, info->snd_portid,
> -					  info->snd_seq, NLM_F_ACK,
> -					  team, true);
> +	nlmsg_free(skb);
> +	return err;
>  }
>  
>  static int team_nl_cmd_port_list_get(struct sk_buff *skb,
> @@ -2406,7 +2430,8 @@ static int team_nl_cmd_port_list_get(struct sk_buff *skb,
>  	if (!team)
>  		return -EINVAL;
>  
> -	err = team_nl_send_generic(info, team, team_nl_fill_port_list_get_all);
> +	err = team_nl_send_port_list_get(team, info->snd_portid, info->snd_seq,
> +					 NLM_F_ACK, team_nl_send_unicast, NULL);
>  
>  	team_nl_team_put(team);
>  
> @@ -2457,27 +2482,11 @@ static int team_nl_send_event_options_get(struct team *team,
>  					sel_opt_inst_list);
>  }
>  
> -static int team_nl_send_event_port_list_get(struct team *team)
> +static int team_nl_send_event_port_get(struct team *team,
> +				       struct team_port *port)
>  {
> -	struct sk_buff *skb;
> -	int err;
> -	struct net *net = dev_net(team->dev);
> -
> -	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> -	if (!skb)
> -		return -ENOMEM;
> -
> -	err = team_nl_fill_port_list_get(skb, 0, 0, 0, team, false);
> -	if (err < 0)
> -		goto err_fill;
> -
> -	err = genlmsg_multicast_netns(net, skb, 0, team_change_event_mcgrp.id,
> -				      GFP_KERNEL);
> -	return err;
> -
> -err_fill:
> -	nlmsg_free(skb);
> -	return err;
> +	return team_nl_send_port_list_get(team, 0, 0, 0, team_nl_send_multicast,
> +					  port);
>  }
>  
>  static int team_nl_init(void)
> @@ -2550,7 +2559,7 @@ static void __team_port_change_send(struct team_port *port, bool linkup)
>  	port->state.duplex = 0;
>  
>  send_event:
> -	err = team_nl_send_event_port_list_get(port->team);
> +	err = team_nl_send_event_port_get(port->team, port);
>  	if (err && err != -ESRCH)
>  		netdev_warn(port->team->dev, "Failed to send port change of device %s via netlink (err %d)\n",
>  			    port->dev->name, err);
> -- 
> 1.8.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Jiri Pirko]

>> +send_done:
>> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
>> +	if (!nlh) {
>> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
>> +		if (err)
>> +			goto errout;
>> +		goto send_done;
>> +	}
>
>I'd suggest to use netlink_dump_start for this, so you don't need to
>manually create the NLMSG_DONE message.
>

I believe that is not possible for genl

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Pablo Neira Ayuso]

On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
> >> +send_done:
> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
> >> +	if (!nlh) {
> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
> >> +		if (err)
> >> +			goto errout;
> >> +		goto send_done;
> >> +	}
> >
> >I'd suggest to use netlink_dump_start for this, so you don't need to
> >manually create the NLMSG_DONE message.
> 
> I believe that is not possible for genl

By looking at net/netlink/genetlink.c, you can make it by means of the
.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
user-space to hit that code.

BTW, perhaps it is good idea to keep the old get function that takes
no NLM_F_DUMP for a while to avoid breaking old versions of userspace
code?

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Jiri Pirko]

Fri, Feb 01, 2013 at 09:43:10PM CET, pablo@netfilter.org wrote:
>On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
>> >> +send_done:
>> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
>> >> +	if (!nlh) {
>> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
>> >> +		if (err)
>> >> +			goto errout;
>> >> +		goto send_done;
>> >> +	}
>> >
>> >I'd suggest to use netlink_dump_start for this, so you don't need to
>> >manually create the NLMSG_DONE message.
>> 
>> I believe that is not possible for genl
>
>By looking at net/netlink/genetlink.c, you can make it by means of the
>.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
>user-space to hit that code.

Correct. But I need to do unsolicited multicast messages as well. And
that can't be resolved by dumpit.

>
>BTW, perhaps it is good idea to keep the old get function that takes
>no NLM_F_DUMP for a while to avoid breaking old versions of userspace
>code?

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Pablo Neira Ayuso]

On Sat, Feb 02, 2013 at 12:31:47AM +0100, Jiri Pirko wrote:
> Fri, Feb 01, 2013 at 09:43:10PM CET, pablo@netfilter.org wrote:
> >On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
> >> >> +send_done:
> >> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
> >> >> +	if (!nlh) {
> >> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
> >> >> +		if (err)
> >> >> +			goto errout;
> >> >> +		goto send_done;
> >> >> +	}
> >> >
> >> >I'd suggest to use netlink_dump_start for this, so you don't need to
> >> >manually create the NLMSG_DONE message.
> >> 
> >> I believe that is not possible for genl
> >
> >By looking at net/netlink/genetlink.c, you can make it by means of the
> >.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
> >user-space to hit that code.
> 
> Correct. But I need to do unsolicited multicast messages as well. And
> that can't be resolved by dumpit.

Interesting. Why do you need multi-part multicast event messages?

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Jiri Pirko]

Sat, Feb 02, 2013 at 02:29:28AM CET, pablo@netfilter.org wrote:
>On Sat, Feb 02, 2013 at 12:31:47AM +0100, Jiri Pirko wrote:
>> Fri, Feb 01, 2013 at 09:43:10PM CET, pablo@netfilter.org wrote:
>> >On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
>> >> >> +send_done:
>> >> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
>> >> >> +	if (!nlh) {
>> >> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
>> >> >> +		if (err)
>> >> >> +			goto errout;
>> >> >> +		goto send_done;
>> >> >> +	}
>> >> >
>> >> >I'd suggest to use netlink_dump_start for this, so you don't need to
>> >> >manually create the NLMSG_DONE message.
>> >> 
>> >> I believe that is not possible for genl
>> >
>> >By looking at net/netlink/genetlink.c, you can make it by means of the
>> >.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
>> >user-space to hit that code.
>> 
>> Correct. But I need to do unsolicited multicast messages as well. And
>> that can't be resolved by dumpit.
>
>Interesting. Why do you need multi-part multicast event messages?

Messages are too big to fit one GENLMSG_DEFAULT_SIZE message.

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Pablo Neira Ayuso]

On Sat, Feb 02, 2013 at 10:04:22AM +0100, Jiri Pirko wrote:
> Sat, Feb 02, 2013 at 02:29:28AM CET, pablo@netfilter.org wrote:
> >On Sat, Feb 02, 2013 at 12:31:47AM +0100, Jiri Pirko wrote:
> >> Fri, Feb 01, 2013 at 09:43:10PM CET, pablo@netfilter.org wrote:
> >> >On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
> >> >> >> +send_done:
> >> >> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
> >> >> >> +	if (!nlh) {
> >> >> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
> >> >> >> +		if (err)
> >> >> >> +			goto errout;
> >> >> >> +		goto send_done;
> >> >> >> +	}
> >> >> >
> >> >> >I'd suggest to use netlink_dump_start for this, so you don't need to
> >> >> >manually create the NLMSG_DONE message.
> >> >> 
> >> >> I believe that is not possible for genl
> >> >
> >> >By looking at net/netlink/genetlink.c, you can make it by means of the
> >> >.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
> >> >user-space to hit that code.
> >> 
> >> Correct. But I need to do unsolicited multicast messages as well. And
> >> that can't be resolved by dumpit.
> >
> >Interesting. Why do you need multi-part multicast event messages?
> 
> Messages are too big to fit one GENLMSG_DEFAULT_SIZE message.

nfnetlink_log may use bigger messages than 1 memory page to deliver
multicast events if configured to include the entire packet
(NFULNL_COPY_PACKET).

I'd suggest to calculate the message size in run-time and allocate
memory you need according to that, several netfilter subsystem are
doing so.

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Jiri Pirko]

Mon, Feb 04, 2013 at 05:03:04PM CET, pablo@netfilter.org wrote:
>On Sat, Feb 02, 2013 at 10:04:22AM +0100, Jiri Pirko wrote:
>> Sat, Feb 02, 2013 at 02:29:28AM CET, pablo@netfilter.org wrote:
>> >On Sat, Feb 02, 2013 at 12:31:47AM +0100, Jiri Pirko wrote:
>> >> Fri, Feb 01, 2013 at 09:43:10PM CET, pablo@netfilter.org wrote:
>> >> >On Fri, Feb 01, 2013 at 08:00:26PM +0100, Jiri Pirko wrote:
>> >> >> >> +send_done:
>> >> >> >> +	nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);
>> >> >> >> +	if (!nlh) {
>> >> >> >> +		err = __send_and_alloc_skb(&skb, team, portid, send_func);
>> >> >> >> +		if (err)
>> >> >> >> +			goto errout;
>> >> >> >> +		goto send_done;
>> >> >> >> +	}
>> >> >> >
>> >> >> >I'd suggest to use netlink_dump_start for this, so you don't need to
>> >> >> >manually create the NLMSG_DONE message.
>> >> >> 
>> >> >> I believe that is not possible for genl
>> >> >
>> >> >By looking at net/netlink/genetlink.c, you can make it by means of the
>> >> >.dumpit callback in struct genl_ops. You have to pass NLM_F_DUMP from
>> >> >user-space to hit that code.
>> >> 
>> >> Correct. But I need to do unsolicited multicast messages as well. And
>> >> that can't be resolved by dumpit.
>> >
>> >Interesting. Why do you need multi-part multicast event messages?
>> 
>> Messages are too big to fit one GENLMSG_DEFAULT_SIZE message.
>
>nfnetlink_log may use bigger messages than 1 memory page to deliver
>multicast events if configured to include the entire packet
>(NFULNL_COPY_PACKET).
>
>I'd suggest to calculate the message size in run-time and allocate
>memory you need according to that, several netfilter subsystem are
>doing so.

I recall I had some issues with longer messages on the userspace part,
I'm not sure what that was exactly. I decided to do this in multipart
messages and in order to do not break userspace I would like to continue
to do so.

Re: [patch net-next v2 1/3] team: handle sending port list in the same way option list is sent

[Pablo Neira Ayuso]

On Mon, Feb 04, 2013 at 05:28:06PM +0100, Jiri Pirko wrote:
> Mon, Feb 04, 2013 at 05:03:04PM CET, pablo@netfilter.org wrote:
> >On Sat, Feb 02, 2013 at 10:04:22AM +0100, Jiri Pirko wrote:
[...]
> >> Messages are too big to fit one GENLMSG_DEFAULT_SIZE message.
> >
> >nfnetlink_log may use bigger messages than 1 memory page to deliver
> >multicast events if configured to include the entire packet
> >(NFULNL_COPY_PACKET).
> >
> >I'd suggest to calculate the message size in run-time and allocate
> >memory you need according to that, several netfilter subsystem are
> >doing so.
> 
> I recall I had some issues with longer messages on the userspace part,
> I'm not sure what that was exactly.

Hm, it should be OK.

> I decided to do this in multipart messages and in order to do not
> break userspace I would like to continue to do so.

It was just a suggestion.

Note that fitting the message in one single skb should also simplify
the handling in case you hit ENOBUFS from user-space. But you know
better your needs for the team driver, I leave this up to you.

[patch net-next v2 2/3] team: move netlink event notifiers after team_port_leave()

[Jiri Pirko]

In team_port_del(), there is need to be do all the cleanup related
things first and netlink event notifiers should be called after that.
This fixes two problems:
team carrier is now correctly set (port is removed from list first)
mode can set option as changed in .port_leave op

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 738f744..694ccf6 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1131,10 +1131,6 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 		return -ENOENT;
 	}
 
-	__team_option_inst_mark_removed_port(team, port);
-	__team_options_change_check(team);
-	__team_option_inst_del_port(team, port);
-	__team_port_change_port_removed(port);
 	team_port_disable(team, port);
 	list_del_rcu(&port->list);
 	netdev_rx_handler_unregister(port_dev);
@@ -1143,6 +1139,12 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 	vlan_vids_del_by_dev(port_dev, dev);
 	dev_close(port_dev);
 	team_port_leave(team, port);
+
+	__team_option_inst_mark_removed_port(team, port);
+	__team_options_change_check(team);
+	__team_option_inst_del_port(team, port);
+	__team_port_change_port_removed(port);
+
 	team_port_set_orig_dev_addr(port);
 	dev_set_mtu(port_dev, port->orig.mtu);
 	synchronize_rcu();
-- 
1.8.1

[patch net-next v2 3/3] team: ab: set active port option as changed when port is leaving

[Jiri Pirko]

In case port is leaving the team, set the option "activeport" as changed
so the change can be properly propagated to userspace

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team_mode_activebackup.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index 6262b4d..40fd338 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -19,6 +19,7 @@
 
 struct ab_priv {
 	struct team_port __rcu *active_port;
+	struct team_option_inst_info *ap_opt_inst_info;
 };
 
 static struct ab_priv *ab_priv(struct team *team)
@@ -54,8 +55,17 @@ drop:
 
 static void ab_port_leave(struct team *team, struct team_port *port)
 {
-	if (ab_priv(team)->active_port == port)
+	if (ab_priv(team)->active_port == port) {
 		RCU_INIT_POINTER(ab_priv(team)->active_port, NULL);
+		team_option_inst_set_change(ab_priv(team)->ap_opt_inst_info);
+	}
+}
+
+static int ab_active_port_init(struct team *team,
+			       struct team_option_inst_info *info)
+{
+	ab_priv(team)->ap_opt_inst_info = info;
+	return 0;
 }
 
 static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx)
@@ -88,6 +98,7 @@ static const struct team_option ab_options[] = {
 	{
 		.name = "activeport",
 		.type = TEAM_OPTION_TYPE_U32,
+		.init = ab_active_port_init,
 		.getter = ab_active_port_get,
 		.setter = ab_active_port_set,
 	},
-- 
1.8.1

Re: [patch net-next v2 0/3] team: couple of patches

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri,  1 Feb 2013 19:17:23 +0100

> Do long time in-todo-queue convert of port list netlik messages to multipart
> ones. That was needed in order to reorder calls in team_port_del(). That
> was needed for correct event send of active port zeroing.
> 
> Jiri Pirko (3):
>   team: handle sending port list in the same way option list is sent
>   team: move netlink event notifiers after team_port_leave()
>   team: ab: set active port option as changed when port is leaving

Series applied, thanks.