[PATCH] Don't send LACP frames when master team device is down.

[Samudrala, Sridhar]

Watch for team admin state changes and update the member ports state
to enable/disable sending LACP frames.

Signed-off-by: Sridhar Samudrala <sridhar.samudrala(a)intel.com&gt;
---
 libteam/ifinfo.c           | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 teamd/teamd.h              |  3 +++
 teamd/teamd_events.c       | 17 +++++++++++++++++
 teamd/teamd_ifinfo_watch.c | 14 ++++++++++----
 teamd/teamd_runner_lacp.c  | 27 +++++++++++++++++++++++++++
 5 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
index fe370e3..47b342c 100644
--- a/libteam/ifinfo.c
+++ b/libteam/ifinfo.c
@@ -57,6 +57,7 @@ struct team_ifinfo {
 	size_t			orig_hwaddr_len;
 	char			ifname[IFNAMSIZ];
 	uint32_t		master_ifindex;
+	bool			admin_state;
 #define MAX_PHYS_PORT_ID_LEN 32
 	char			phys_port_id[MAX_PHYS_PORT_ID_LEN];
 	size_t			phys_port_id_len;
@@ -70,10 +71,11 @@ struct team_ifinfo {
 #define CHANGED_MASTER_IFINDEX		(1 << 4)
 #define CHANGED_PHYS_PORT_ID		(1 << 5)
 #define CHANGED_PHYS_PORT_ID_LEN	(1 << 6)
+#define CHANGED_ADMIN_STATE		(1 << 7)
 #define CHANGED_ANY	(CHANGED_REMOVED | CHANGED_HWADDR | \
 			 CHANGED_HWADDR_LEN | CHANGED_IFNAME | \
 			 CHANGED_MASTER_IFINDEX | CHANGED_PHYS_PORT_ID | \
-			 CHANGED_PHYS_PORT_ID_LEN)
+			 CHANGED_PHYS_PORT_ID_LEN | CHANGED_ADMIN_STATE)
 
 static void set_changed(struct team_ifinfo *ifinfo, int bit)
 {
@@ -127,6 +129,20 @@ static void update_ifname(struct team_ifinfo *ifinfo, struct
rtnl_link *link)
 	}
 }
 
+static void update_admin_state(struct team_ifinfo *ifinfo, struct rtnl_link *link)
+{
+	unsigned int flags;
+	bool admin_state;
+
+	flags = rtnl_link_get_flags(link);
+	admin_state = ((flags & IFF_UP) == IFF_UP);
+
+	if (admin_state != ifinfo->admin_state) {
+		ifinfo->admin_state = admin_state;
+		set_changed(ifinfo, CHANGED_ADMIN_STATE);
+	}
+}
+
 static void update_master(struct team_ifinfo *ifinfo, struct rtnl_link *link)
 {
 	uint32_t master_ifindex;
@@ -172,6 +188,7 @@ static void ifinfo_update(struct team_ifinfo *ifinfo, struct rtnl_link
*link)
 	update_master(ifinfo, link);
 	update_hwaddr(ifinfo, link);
 	update_phys_port_id(ifinfo, link);
+	update_admin_state(ifinfo, link);
 }
 
 static struct team_ifinfo *ifinfo_find(struct team_handle *th, uint32_t ifindex)
@@ -475,6 +492,19 @@ uint32_t team_get_ifinfo_ifindex(struct team_ifinfo *ifinfo)
 /**
  * @param ifinfo	ifinfo structure
  *
+ * @details Get ifinfo admin state.
+ *
+ * @return Ifinfo interface index as idenfified by in kernel.
+ **/
+TEAM_EXPORT
+bool team_get_ifinfo_admin_state(struct team_ifinfo *ifinfo)
+{
+	return ifinfo->admin_state;
+}
+
+/**
+ * @param ifinfo	ifinfo structure
+ *
  * @details Get port associated to rtnetlink interface info.
  *
  * @return Pointer to appropriate team_port structure
@@ -619,6 +649,19 @@ bool team_is_ifinfo_master_ifindex_changed(struct team_ifinfo
*ifinfo)
 /**
  * @param ifinfo	ifinfo structure
  *
+ * @details See if admin state of interface got changed.
+ *
+ * @return True if admin state of interface got changed.
+ **/
+TEAM_EXPORT
+bool team_is_ifinfo_admin_state_changed(struct team_ifinfo *ifinfo)
+{
+	return is_changed(ifinfo, CHANGED_ADMIN_STATE);
+}
+
+/**
+ * @param ifinfo	ifinfo structure
+ *
  * @details Get ifinfo physical port ID.
  *
  * @return Pointer to memory place where physical por ID is.
diff --git a/teamd/teamd.h b/teamd/teamd.h
index 344d188..845ca18 100644
--- a/teamd/teamd.h
+++ b/teamd/teamd.h
@@ -174,6 +174,7 @@ struct teamd_runner {
 struct teamd_event_watch_ops {
 	int (*hwaddr_changed)(struct teamd_context *ctx, void *priv);
 	int (*ifname_changed)(struct teamd_context *ctx, void *priv);
+	int (*admin_state_changed)(struct teamd_context *ctx, void *priv);
 	int (*port_added)(struct teamd_context *ctx,
 			  struct teamd_port *tdport, void *priv);
 	void (*port_removed)(struct teamd_context *ctx,
@@ -205,6 +206,8 @@ int teamd_event_ifinfo_hwaddr_changed(struct teamd_context *ctx,
 				      struct team_ifinfo *ifinfo);
 int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx,
 				      struct team_ifinfo *ifinfo);
+int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx,
+					   struct team_ifinfo *ifinfo);
 int teamd_events_init(struct teamd_context *ctx);
 void teamd_events_fini(struct teamd_context *ctx);
 int teamd_event_watch_register(struct teamd_context *ctx,
diff --git a/teamd/teamd_events.c b/teamd/teamd_events.c
index f2805fb..1a95974 100644
--- a/teamd/teamd_events.c
+++ b/teamd/teamd_events.c
@@ -167,6 +167,23 @@ int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx,
 	return 0;
 }
 
+int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx,
+					   struct team_ifinfo *ifinfo)
+{
+	struct event_watch_item *watch;
+	uint32_t ifindex = team_get_ifinfo_ifindex(ifinfo);
+	int err;
+
+	list_for_each_node_entry(watch, &ctx->event_watch_list, list) {
+		if (watch->ops->admin_state_changed && ctx->ifindex == ifindex) {
+			err = watch->ops->admin_state_changed(ctx, watch->priv);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
 int teamd_events_init(struct teamd_context *ctx)
 {
 	list_init(&ctx->event_watch_list);
diff --git a/teamd/teamd_ifinfo_watch.c b/teamd/teamd_ifinfo_watch.c
index bdd3b8d..f334ff6 100644
--- a/teamd/teamd_ifinfo_watch.c
+++ b/teamd/teamd_ifinfo_watch.c
@@ -36,10 +36,16 @@ static int ifinfo_change_handler_func(struct team_handle *th, void
*priv,
 	int err;
 
 	team_for_each_ifinfo(ifinfo, th) {
-		if (ctx->ifinfo == ifinfo &&
-		    team_is_ifinfo_removed(ifinfo)) {
-			teamd_log_warn("Team device removal detected.");
-			teamd_run_loop_quit(ctx, 0);
+		if (ctx->ifinfo == ifinfo) {
+			if (team_is_ifinfo_removed(ifinfo)) {
+				teamd_log_warn("Team device removal detected.");
+				teamd_run_loop_quit(ctx, 0);
+			}
+			if (team_is_ifinfo_admin_state_changed(ifinfo)) {
+				err = teamd_event_ifinfo_admin_state_changed(ctx, ifinfo);
+				if (err)
+					return err;
+			}
 		}
 
 		if (team_is_ifinfo_hwaddr_changed(ifinfo) ||
diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
index 59d505d..8d88e87 100644
--- a/teamd/teamd_runner_lacp.c
+++ b/teamd/teamd_runner_lacp.c
@@ -1016,6 +1016,11 @@ static int lacpdu_send(struct lacp_port *lacp_port)
 	struct sockaddr_ll ll_my;
 	struct sockaddr_ll ll_slow;
 	int err;
+	bool admin_state;
+
+	admin_state = team_get_ifinfo_admin_state(lacp_port->ctx->ifinfo);
+	if (!admin_state)
+		return 0;
 
 	err = teamd_getsockname_hwaddr(lacp_port->sock, &ll_my, 0);
 	if (err)
@@ -1302,6 +1307,27 @@ static int lacp_event_watch_hwaddr_changed(struct teamd_context
*ctx,
 	return 0;
 }
 
+static int lacp_event_watch_admin_state_changed(struct teamd_context *ctx,
+					        void *priv)
+{
+	struct lacp *lacp = priv;
+	struct teamd_port *tdport;
+	bool admin_state;
+	int err;
+
+	teamd_for_each_tdport(tdport, ctx) {
+		struct lacp_port *lacp_port = lacp_port_get(lacp, tdport);
+
+		admin_state = team_get_ifinfo_admin_state(ctx->ifinfo);
+		err = lacp_port_set_state(lacp_port,
+					  admin_state?PORT_STATE_CURRENT:PORT_STATE_DISABLED);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+
 static int lacp_event_watch_port_added(struct teamd_context *ctx,
 				       struct teamd_port *tdport, void *priv)
 {
@@ -1336,6 +1362,7 @@ static const struct teamd_event_watch_ops lacp_port_watch_ops = {
 	.port_added = lacp_event_watch_port_added,
 	.port_removed = lacp_event_watch_port_removed,
 	.port_changed = lacp_event_watch_port_changed,
+	.admin_state_changed = lacp_event_watch_admin_state_changed,
 };
 
 static int lacp_carrier_init(struct teamd_context *ctx, struct lacp *lacp)
-- 
2.1.0

[Jiri Pirko]

Wed, Sep 02, 2015 at 12:54:54AM CEST, sridhar.samudrala(a)intel.com wrote:
> Watch for team admin state changes and update the member ports state
> to enable/disable sending LACP frames.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala(a)intel.com&gt;
> ---
>  libteam/ifinfo.c           | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>  teamd/teamd.h              |  3 +++
>  teamd/teamd_events.c       | 17 +++++++++++++++++
>  teamd/teamd_ifinfo_watch.c | 14 ++++++++++----
>  teamd/teamd_runner_lacp.c  | 27 +++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+), 5 deletions(-) 
<snip>
	

Patch looks good to me. I think that lacp part should be a separate
patch.


> diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> index 59d505d..8d88e87 100644
> --- a/teamd/teamd_runner_lacp.c
> +++ b/teamd/teamd_runner_lacp.c
> @@ -1016,6 +1016,11 @@ static int lacpdu_send(struct lacp_port *lacp_port)
>  	struct sockaddr_ll ll_my;
>  	struct sockaddr_ll ll_slow;
>  	int err;
> +	bool admin_state;
> +
> +	admin_state = team_get_ifinfo_admin_state(lacp_port->ctx->ifinfo);
> +	if (!admin_state)
> +		return 0;
>  
>  	err = teamd_getsockname_hwaddr(lacp_port->sock, &ll_my, 0);
>  	if (err)
> @@ -1302,6 +1307,27 @@ static int lacp_event_watch_hwaddr_changed(struct teamd_context
> *ctx,
>  	return 0;
>  }
>  
> +static int lacp_event_watch_admin_state_changed(struct teamd_context *ctx,
> +					        void *priv)
> +{
> +	struct lacp *lacp = priv;
> +	struct teamd_port *tdport;
> +	bool admin_state;
> +	int err;
> +
> +	teamd_for_each_tdport(tdport, ctx) {
> +		struct lacp_port *lacp_port = lacp_port_get(lacp, tdport);
> +
> +		admin_state = team_get_ifinfo_admin_state(ctx->ifinfo);
> +		err = lacp_port_set_state(lacp_port,
> +					  admin_state?PORT_STATE_CURRENT:PORT_STATE_DISABLED);
> +		if (err)
> +			return err;
> +	}
> +	return 0;
> +}
> +
> +
>  static int lacp_event_watch_port_added(struct teamd_context *ctx,
>  				       struct teamd_port *tdport, void *priv)
>  {
> @@ -1336,6 +1362,7 @@ static const struct teamd_event_watch_ops lacp_port_watch_ops = {
I think that this struct should be renamed to lacp_event_watch_ops, sice
this this no longer about port events only.


Thanks for taking care of this!


>  	.port_added = lacp_event_watch_port_added,
>  	.port_removed = lacp_event_watch_port_removed,
>  	.port_changed = lacp_event_watch_port_changed,
> +	.admin_state_changed = lacp_event_watch_admin_state_changed,
>  };
>  
>  static int lacp_carrier_init(struct teamd_context *ctx, struct lacp *lacp)
> -- 
> 2.1.0
>

[Samudrala, Sridhar]

On 9/2/2015 12:15 AM, Jiri Pirko wrote:
>  Wed, Sep 02, 2015 at 12:54:54AM CEST, sridhar.samudrala(a)intel.com
> wrote:
> > Watch for team admin state changes and update the member ports state
> > to enable/disable sending LACP frames.
> >
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala(a)intel.com&gt;
> > ---
> > libteam/ifinfo.c           | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> > teamd/teamd.h              |  3 +++
> > teamd/teamd_events.c       | 17 +++++++++++++++++
> > teamd/teamd_ifinfo_watch.c | 14 ++++++++++----
> > teamd/teamd_runner_lacp.c  | 27 +++++++++++++++++++++++++++
> > 5 files changed, 101 insertions(+), 5 deletions(-)
>  <snip>
>  	
>
>  Patch looks good to me. I think that lacp part should be a separate
>  patch.
>
>  
OK. Will resubmit as 2 patches with struct change to lacp_event_watch_ops.

-Sridhar