[patch libteam] use original hwaddr as source address in lacpdus

[Eric Kinzie]

The team lacp runner sets the mac addresses of all member interfaces
  to be the same value.  However, LACPDUs should carry an interface's
  original (individual) MAC address.

Signed-off-by: Eric Kinzie 
---
 teamd/teamd.h             |  5 +++++
 teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
 teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
 3 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/teamd/teamd.h b/teamd/teamd.h
index d8ab6c8..5dbfb9b 100644
--- a/teamd/teamd.h
+++ b/teamd/teamd.h
@@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,

int teamd_hash_func_set(struct teamd_context *ctx);

+int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
+				const unsigned short family,
+				const struct sock_fprog *fprog,
+				const struct sock_fprog *alt_fprog);
 int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
    		   const unsigned short family,
    		   const struct sock_fprog *fprog,
@@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
    		     size_t expected_len);
 int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
    	 const struct sockaddr *dest_addr, socklen_t addrlen);
+int teamd_send(int sockfd, const void *buf, size_t len, int flags);
 int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
    	   struct sockaddr *src_addr, socklen_t addrlen);

diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
index 337cfbf..f32c2ef 100644
--- a/teamd/teamd_common.c
+++ b/teamd/teamd_common.c
@@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
    return 0;
 }

-int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
-			   const unsigned short family,
-			   const struct sock_fprog *fprog,
-			   const struct sock_fprog *alt_fprog)
+int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
+				const unsigned short family,
+				const struct sock_fprog *fprog,
+				const struct sock_fprog *alt_fprog)
 {
    struct sockaddr_ll ll_my;
    int sock;
    int ret;
    int err;

-	sock = socket(PF_PACKET, SOCK_DGRAM, 0);
+	sock = socket(PF_PACKET, type, 0);
    if (sock == -1) {
    	teamd_log_err("Failed to create packet socket.");
    	return -errno;
@@ -121,6 +121,15 @@ close_sock:
    return err;
 }

+int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
+			   const unsigned short family,
+			   const struct sock_fprog *fprog,
+			   const struct sock_fprog *alt_fprog)
+{
+	return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
+					   fprog, alt_fprog);
+}
+
 int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
    		     size_t expected_len)
 {
@@ -164,6 +173,29 @@ resend:
    return 0;
 }

+int teamd_send(int sockfd, const void *buf, size_t len, int flags)
+{
+	ssize_t ret;
+
+resend:
+	ret = send(sockfd, buf, len, flags);
+	if (ret == -1) {
+		switch(errno) {
+		case EINTR:
+			goto resend;
+		case ENETDOWN:
+		case ENETUNREACH:
+		case EADDRNOTAVAIL:
+		case ENXIO:
+			return 0;
+		default:
+			teamd_log_err("send failed.");
+			return -errno;
+		}
+	}
+	return 0;
+}
+
 int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
    	   struct sockaddr *src_addr, socklen_t addrlen)
 {
diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
index 1af3b17..dc03996 100644
--- a/teamd/teamd_runner_lacp.c
+++ b/teamd/teamd_runner_lacp.c
@@ -30,6 +30,7 @@
 #include <errno.h>
 #include <team.h>
 #include <private/misc.h>
+#include <net/ethernet.h>

#include "teamd.h"
 #include "teamd_config.h"
@@ -60,6 +61,7 @@ struct lacpdu_info {
 } __attribute__((__packed__));

struct lacpdu {
+	struct ether_header	hdr;
    uint8_t			subtype;
    uint8_t			version_number;
    uint8_t			actor_tlv_type;
@@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
    struct lacpdu lacpdu;
    struct sockaddr_ll ll_my;
    struct sockaddr_ll ll_slow;
+	char *hwaddr;
+	unsigned char hwaddr_len;
    int err;
    bool admin_state;

@@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)

memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);

+	hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
+	hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
+	if (hwaddr_len != ETH_ALEN)
+		return 0;
+
    lacpdu_init(&lacpdu);
    lacpdu.actor = lacp_port->actor;
    lacpdu.partner = lacp_port->partner;
+	memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
+	memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
+	lacpdu.hdr.ether_type = htons(ETH_P_SLOW);

-	err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
-			   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
+	err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
    return err;
 }

@@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
    	return err;
    }

-	err = teamd_packet_sock_open(&lacp_port->sock,
-				     tdport->ifindex,
-				     htons(ETH_P_SLOW), NULL, NULL);
+	err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
+					  tdport->ifindex,
+					  htons(ETH_P_SLOW), NULL, NULL);
    if (err)
    	return err;

[Jiri Pirko]

Fri, Apr 15, 2016 at 01:14:04AM CEST, [e-mail] wrote:
> The team lacp runner sets the mac addresses of all member interfaces
>  to be the same value.  However, LACPDUs should carry an interface's
>  original (individual) MAC address.

^^
Apart these useless spaces, the patch itself looks good to me. 43.4.2.2
really says: "Source Address (SA). The SA in LACPDUs carries the
individual MAC address associated with the port through which the
LACPDU is transmitted." But it does not say anything about original port
mac, and the current port mac is the one of the team master. Why is this
change needed? Does any aggregator actually care what src lacpdu address
is?
> Signed-off-by: Eric Kinzie
>
> teamd/teamd.h             |  5 +++++
> teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
> teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
> 3 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/teamd/teamd.h b/teamd/teamd.h
> index d8ab6c8..5dbfb9b 100644
> --- a/teamd/teamd.h
> +++ b/teamd/teamd.h
> @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
>
> int teamd_hash_func_set(struct teamd_context *ctx);
>
> +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> 		const unsigned short family,
> 		const struct sock_fprog *fprog,
> 		const struct sock_fprog *alt_fprog);
>
> int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
>    		   const unsigned short family,
>    		   const struct sock_fprog *fprog,
> @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
>    		     size_t expected_len);
> int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
>    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
> int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
>    	   struct sockaddr *src_addr, socklen_t addrlen);
>
> diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> index 337cfbf..f32c2ef 100644
> --- a/teamd/teamd_common.c
> +++ b/teamd/teamd_common.c
> @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
>    return 0;
> }
>
> -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> 	   const unsigned short family,
> 	   const struct sock_fprog *fprog,
> 	   const struct sock_fprog *alt_fprog)
>
> +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> 		const unsigned short family,
> 		const struct sock_fprog *fprog,
> 		const struct sock_fprog *alt_fprog)
>
> {
>    struct sockaddr_ll ll_my;
>    int sock;
>    int ret;
>    int err;
> sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> sock = socket(PF_PACKET, type, 0);
> if (sock == -1) {
> teamd_log_err("Failed to create packet socket.");
> return -errno;
>
> @@ -121,6 +121,15 @@ close_sock:
>    return err;
> }
>
> +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> 	   const unsigned short family,
> 	   const struct sock_fprog *fprog,
> 	   const struct sock_fprog *alt_fprog)
>
> +{
> return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> 			   fprog, alt_fprog);
>
> +}
>
> int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
>    		     size_t expected_len)
> {
> @@ -164,6 +173,29 @@ resend:
>    return 0;
> }
>
> +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> +{
> ssize_t ret;
>
> +resend:
> ret = send(sockfd, buf, len, flags);
> if (ret == -1) {
> switch(errno) {
> case EINTR:
> 	goto resend;
> case ENETDOWN:
> case ENETUNREACH:
> case EADDRNOTAVAIL:
> case ENXIO:
> 	return 0;
> default:
> 	teamd_log_err("send failed.");
> 	return -errno;
> }
> }
> return 0;
>
> +}
>
> int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
>    	   struct sockaddr *src_addr, socklen_t addrlen)
> {
> diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> index 1af3b17..dc03996 100644
> --- a/teamd/teamd_runner_lacp.c
> +++ b/teamd/teamd_runner_lacp.c
> @@ -30,6 +30,7 @@
> #include <errno.h>
> #include <team.h>
> #include <private/misc.h>
> +#include <net/ethernet.h>
>
> #include "teamd.h"
> #include "teamd_config.h"
> @@ -60,6 +61,7 @@ struct lacpdu_info {
> } __attribute__((__packed__));
>
> struct lacpdu {
> struct ether_header	hdr;
> uint8_t			subtype;
> uint8_t			version_number;
> uint8_t			actor_tlv_type;
>
> @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
>    struct lacpdu lacpdu;
>    struct sockaddr_ll ll_my;
>    struct sockaddr_ll ll_slow;
> char *hwaddr;
> unsigned char hwaddr_len;
> int err;
> bool admin_state;
>
> @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
>
> memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> if (hwaddr_len != ETH_ALEN)
> return 0;
> lacpdu_init(&lacpdu);
>  lacpdu.actor = lacp_port->actor;
>  lacpdu.partner = lacp_port->partner;
> memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> return err;
>
> }
>
> @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
>    	return err;
>    }
> err = teamd_packet_sock_open(&lacp_port->sock,
> 		     tdport->ifindex,
> 		     htons(ETH_P_SLOW), NULL, NULL);
> err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> 			  tdport->ifindex,
> 			  htons(ETH_P_SLOW), NULL, NULL);
> if (err)
> return err;
>
> -- 
> 2.1.4

[Flavio Leitner]

On Fri, Apr 15, 2016 at 08:27:02AM +0200, Jiri Pirko wrote:
> Fri, Apr 15, 2016 at 01:14:04AM CEST, [e-mail] wrote:
> > The team lacp runner sets the mac addresses of all member interfaces
> >  to be the same value.  However, LACPDUs should carry an interface's
> >  original (individual) MAC address.
>
> ^^
> Apart these useless spaces, the patch itself looks good to me. 43.4.2.2
> really says: "Source Address (SA). The SA in LACPDUs carries the
> individual MAC address associated with the port through which the
> LACPDU is transmitted." But it does not say anything about original port
> mac, and the current port mac is the one of the team master. Why is this
> change needed? Does any aggregator actually care what src lacpdu address
> is?

It's been some time already, but I recall about looping prevention.
If you get LACPDUs coming from yourself, it means there is a loop
in the network.

fbl
> > Signed-off-by: Eric Kinzie
> >
> > teamd/teamd.h             |  5 +++++
> > teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
> > teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
> > 3 files changed, 58 insertions(+), 10 deletions(-)
> >
> > diff --git a/teamd/teamd.h b/teamd/teamd.h
> > index d8ab6c8..5dbfb9b 100644
> > --- a/teamd/teamd.h
> > +++ b/teamd/teamd.h
> > @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
> >
> > int teamd_hash_func_set(struct teamd_context *ctx);
> >
> > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > 		const unsigned short family,
> > 		const struct sock_fprog *fprog,
> > 		const struct sock_fprog *alt_fprog);
> >
> > int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> >    		   const unsigned short family,
> >    		   const struct sock_fprog *fprog,
> > @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> >    		     size_t expected_len);
> > int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
> >    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> > +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
> > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> >    	   struct sockaddr *src_addr, socklen_t addrlen);
> >
> > diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> > index 337cfbf..f32c2ef 100644
> > --- a/teamd/teamd_common.c
> > +++ b/teamd/teamd_common.c
> > @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
> >    return 0;
> > }
> >
> > -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > 	   const unsigned short family,
> > 	   const struct sock_fprog *fprog,
> > 	   const struct sock_fprog *alt_fprog)
> >
> > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > 		const unsigned short family,
> > 		const struct sock_fprog *fprog,
> > 		const struct sock_fprog *alt_fprog)
> >
> > {
> >    struct sockaddr_ll ll_my;
> >    int sock;
> >    int ret;
> >    int err;
> > sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> > sock = socket(PF_PACKET, type, 0);
> > if (sock == -1) {
> > teamd_log_err("Failed to create packet socket.");
> > return -errno;
> >
> > @@ -121,6 +121,15 @@ close_sock:
> >    return err;
> > }
> >
> > +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > 	   const unsigned short family,
> > 	   const struct sock_fprog *fprog,
> > 	   const struct sock_fprog *alt_fprog)
> >
> > +{
> > return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> > 			   fprog, alt_fprog);
> >
> > +}
> >
> > int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> >    		     size_t expected_len)
> > {
> > @@ -164,6 +173,29 @@ resend:
> >    return 0;
> > }
> >
> > +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> > +{
> > ssize_t ret;
> >
> > +resend:
> > ret = send(sockfd, buf, len, flags);
> > if (ret == -1) {
> > switch(errno) {
> > case EINTR:
> > 	goto resend;
> > case ENETDOWN:
> > case ENETUNREACH:
> > case EADDRNOTAVAIL:
> > case ENXIO:
> > 	return 0;
> > default:
> > 	teamd_log_err("send failed.");
> > 	return -errno;
> > }
> > }
> > return 0;
> >
> > +}
> >
> > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> >    	   struct sockaddr *src_addr, socklen_t addrlen)
> > {
> > diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> > index 1af3b17..dc03996 100644
> > --- a/teamd/teamd_runner_lacp.c
> > +++ b/teamd/teamd_runner_lacp.c
> > @@ -30,6 +30,7 @@
> > #include <errno.h>
> > #include <team.h>
> > #include <private/misc.h>
> > +#include <net/ethernet.h>
> >
> > #include "teamd.h"
> > #include "teamd_config.h"
> > @@ -60,6 +61,7 @@ struct lacpdu_info {
> > } __attribute__((__packed__));
> >
> > struct lacpdu {
> > struct ether_header	hdr;
> > uint8_t			subtype;
> > uint8_t			version_number;
> > uint8_t			actor_tlv_type;
> >
> > @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> >    struct lacpdu lacpdu;
> >    struct sockaddr_ll ll_my;
> >    struct sockaddr_ll ll_slow;
> > char *hwaddr;
> > unsigned char hwaddr_len;
> > int err;
> > bool admin_state;
> >
> > @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> >
> > memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> > hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> > hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> > if (hwaddr_len != ETH_ALEN)
> > return 0;
> > lacpdu_init(&lacpdu);
> >  lacpdu.actor = lacp_port->actor;
> >  lacpdu.partner = lacp_port->partner;
> > memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> > memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> > lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> > err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> > 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> > err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> > return err;
> >
> > }
> >
> > @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
> >    	return err;
> >    }
> > err = teamd_packet_sock_open(&lacp_port->sock,
> > 		     tdport->ifindex,
> > 		     htons(ETH_P_SLOW), NULL, NULL);
> > err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> > 			  tdport->ifindex,
> > 			  htons(ETH_P_SLOW), NULL, NULL);
> > if (err)
> > return err;
> >
> > -- 
> > 2.1.4
>
> libteam mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]

[Jiri Pirko]

Fri, Apr 15, 2016 at 03:50:06PM CEST, [e-mail] wrote:
> On Fri, Apr 15, 2016 at 08:27:02AM +0200, Jiri Pirko wrote:
> > Fri, Apr 15, 2016 at 01:14:04AM CEST, [e-mail] wrote:
> > > The team lacp runner sets the mac addresses of all member interfaces
> > >  to be the same value.  However, LACPDUs should carry an interface's
> > >  original (individual) MAC address.
> >
> > ^^
> > Apart these useless spaces, the patch itself looks good to me. 43.4.2.2
> > really says: "Source Address (SA). The SA in LACPDUs carries the
> > individual MAC address associated with the port through which the
> > LACPDU is transmitted." But it does not say anything about original port
> > mac, and the current port mac is the one of the team master. Why is this
> > change needed? Does any aggregator actually care what src lacpdu address
> > is?
>
> It's been some time already, but I recall about looping prevention.
> If you get LACPDUs coming from yourself, it means there is a loop
> in the network.

But that work with team mac as well. No need to use port oric mac for
that.
> fbl
> > > Signed-off-by: Eric Kinzie
> > >
> > > teamd/teamd.h             |  5 +++++
> > > teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
> > > teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
> > > 3 files changed, 58 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/teamd/teamd.h b/teamd/teamd.h
> > > index d8ab6c8..5dbfb9b 100644
> > > --- a/teamd/teamd.h
> > > +++ b/teamd/teamd.h
> > > @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
> > >
> > > int teamd_hash_func_set(struct teamd_context *ctx);
> > >
> > > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > > 		const unsigned short family,
> > > 		const struct sock_fprog *fprog,
> > > 		const struct sock_fprog *alt_fprog);
> > >
> > > int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > >    		   const unsigned short family,
> > >    		   const struct sock_fprog *fprog,
> > > @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> > >    		     size_t expected_len);
> > > int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
> > >    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> > > +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
> > > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> > >    	   struct sockaddr *src_addr, socklen_t addrlen);
> > >
> > > diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> > > index 337cfbf..f32c2ef 100644
> > > --- a/teamd/teamd_common.c
> > > +++ b/teamd/teamd_common.c
> > > @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
> > >    return 0;
> > > }
> > >
> > > -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > > 	   const unsigned short family,
> > > 	   const struct sock_fprog *fprog,
> > > 	   const struct sock_fprog *alt_fprog)
> > >
> > > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > > 		const unsigned short family,
> > > 		const struct sock_fprog *fprog,
> > > 		const struct sock_fprog *alt_fprog)
> > >
> > > {
> > >    struct sockaddr_ll ll_my;
> > >    int sock;
> > >    int ret;
> > >    int err;
> > > sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> > > sock = socket(PF_PACKET, type, 0);
> > > if (sock == -1) {
> > > teamd_log_err("Failed to create packet socket.");
> > > return -errno;
> > >
> > > @@ -121,6 +121,15 @@ close_sock:
> > >    return err;
> > > }
> > >
> > > +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > > 	   const unsigned short family,
> > > 	   const struct sock_fprog *fprog,
> > > 	   const struct sock_fprog *alt_fprog)
> > >
> > > +{
> > > return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> > > 			   fprog, alt_fprog);
> > >
> > > +}
> > >
> > > int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> > >    		     size_t expected_len)
> > > {
> > > @@ -164,6 +173,29 @@ resend:
> > >    return 0;
> > > }
> > >
> > > +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> > > +{
> > > ssize_t ret;
> > >
> > > +resend:
> > > ret = send(sockfd, buf, len, flags);
> > > if (ret == -1) {
> > > switch(errno) {
> > > case EINTR:
> > > 	goto resend;
> > > case ENETDOWN:
> > > case ENETUNREACH:
> > > case EADDRNOTAVAIL:
> > > case ENXIO:
> > > 	return 0;
> > > default:
> > > 	teamd_log_err("send failed.");
> > > 	return -errno;
> > > }
> > > }
> > > return 0;
> > >
> > > +}
> > >
> > > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> > >    	   struct sockaddr *src_addr, socklen_t addrlen)
> > > {
> > > diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> > > index 1af3b17..dc03996 100644
> > > --- a/teamd/teamd_runner_lacp.c
> > > +++ b/teamd/teamd_runner_lacp.c
> > > @@ -30,6 +30,7 @@
> > > #include <errno.h>
> > > #include <team.h>
> > > #include <private/misc.h>
> > > +#include <net/ethernet.h>
> > >
> > > #include "teamd.h"
> > > #include "teamd_config.h"
> > > @@ -60,6 +61,7 @@ struct lacpdu_info {
> > > } __attribute__((__packed__));
> > >
> > > struct lacpdu {
> > > struct ether_header	hdr;
> > > uint8_t			subtype;
> > > uint8_t			version_number;
> > > uint8_t			actor_tlv_type;
> > >
> > > @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> > >    struct lacpdu lacpdu;
> > >    struct sockaddr_ll ll_my;
> > >    struct sockaddr_ll ll_slow;
> > > char *hwaddr;
> > > unsigned char hwaddr_len;
> > > int err;
> > > bool admin_state;
> > >
> > > @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> > >
> > > memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> > > hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> > > hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> > > if (hwaddr_len != ETH_ALEN)
> > > return 0;
> > > lacpdu_init(&lacpdu);
> > >  lacpdu.actor = lacp_port->actor;
> > >  lacpdu.partner = lacp_port->partner;
> > > memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> > > memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> > > lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> > > err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> > > 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> > > err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> > > return err;
> > >
> > > }
> > >
> > > @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
> > >    	return err;
> > >    }
> > > err = teamd_packet_sock_open(&lacp_port->sock,
> > > 		     tdport->ifindex,
> > > 		     htons(ETH_P_SLOW), NULL, NULL);
> > > err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> > > 			  tdport->ifindex,
> > > 			  htons(ETH_P_SLOW), NULL, NULL);
> > > if (err)
> > > return err;
> > >
> > > -- 
> > > 2.1.4
> >
> > libteam mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/admin/lists/[e-mail]
>
> -- 
> fbl

[Eric Kinzie]

On Fri Apr 15 08:27:02 +0200 2016, Jiri Pirko wrote:
> Fri, Apr 15, 2016 at 01:14:04AM CEST, [e-mail] wrote:
> > The team lacp runner sets the mac addresses of all member interfaces
> >  to be the same value.  However, LACPDUs should carry an interface's
> >  original (individual) MAC address.
>
> ^^
> Apart these useless spaces, the patch itself looks good to me. 43.4.2.2
> really says: "Source Address (SA). The SA in LACPDUs carries the
> individual MAC address associated with the port through which the
> LACPDU is transmitted." But it does not say anything about original port
> mac, and the current port mac is the one of the team master. Why is this
> change needed? Does any aggregator actually care what src lacpdu address
> is?

The Addressing section (5.2.10 in 802.1ax) explains this a little better:
    Protocol entities sourcing frames from within the Link Aggregation
    sublayer (e.g., LACP and the Marker protocol) use the MAC address
    of the MAC within an underlying port as the source address in
    frames transmitted through that port.  The MAC Client sees only
    the Aggregator and not the underlying MACs, and therefore uses
    the Aggregator's MAC address as the source address in transmitted
    frames.

I haven't found any implementations that are picky about the source
address.  However I think this change more closely matches the specified
behavior.  Also, I find it helpful during debugging since it's immediately
obvious which interface sent a control packet.

Eric
> > Signed-off-by: Eric Kinzie
> >
> > teamd/teamd.h             |  5 +++++
> > teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
> > teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
> > 3 files changed, 58 insertions(+), 10 deletions(-)
> >
> > diff --git a/teamd/teamd.h b/teamd/teamd.h
> > index d8ab6c8..5dbfb9b 100644
> > --- a/teamd/teamd.h
> > +++ b/teamd/teamd.h
> > @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
> >
> > int teamd_hash_func_set(struct teamd_context *ctx);
> >
> > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > 		const unsigned short family,
> > 		const struct sock_fprog *fprog,
> > 		const struct sock_fprog *alt_fprog);
> >
> > int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> >    		   const unsigned short family,
> >    		   const struct sock_fprog *fprog,
> > @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> >    		     size_t expected_len);
> > int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
> >    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> > +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
> > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> >    	   struct sockaddr *src_addr, socklen_t addrlen);
> >
> > diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> > index 337cfbf..f32c2ef 100644
> > --- a/teamd/teamd_common.c
> > +++ b/teamd/teamd_common.c
> > @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
> >    return 0;
> > }
> >
> > -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > 	   const unsigned short family,
> > 	   const struct sock_fprog *fprog,
> > 	   const struct sock_fprog *alt_fprog)
> >
> > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > 		const unsigned short family,
> > 		const struct sock_fprog *fprog,
> > 		const struct sock_fprog *alt_fprog)
> >
> > {
> >    struct sockaddr_ll ll_my;
> >    int sock;
> >    int ret;
> >    int err;
> > sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> > sock = socket(PF_PACKET, type, 0);
> > if (sock == -1) {
> > teamd_log_err("Failed to create packet socket.");
> > return -errno;
> >
> > @@ -121,6 +121,15 @@ close_sock:
> >    return err;
> > }
> >
> > +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > 	   const unsigned short family,
> > 	   const struct sock_fprog *fprog,
> > 	   const struct sock_fprog *alt_fprog)
> >
> > +{
> > return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> > 			   fprog, alt_fprog);
> >
> > +}
> >
> > int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> >    		     size_t expected_len)
> > {
> > @@ -164,6 +173,29 @@ resend:
> >    return 0;
> > }
> >
> > +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> > +{
> > ssize_t ret;
> >
> > +resend:
> > ret = send(sockfd, buf, len, flags);
> > if (ret == -1) {
> > switch(errno) {
> > case EINTR:
> > 	goto resend;
> > case ENETDOWN:
> > case ENETUNREACH:
> > case EADDRNOTAVAIL:
> > case ENXIO:
> > 	return 0;
> > default:
> > 	teamd_log_err("send failed.");
> > 	return -errno;
> > }
> > }
> > return 0;
> >
> > +}
> >
> > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> >    	   struct sockaddr *src_addr, socklen_t addrlen)
> > {
> > diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> > index 1af3b17..dc03996 100644
> > --- a/teamd/teamd_runner_lacp.c
> > +++ b/teamd/teamd_runner_lacp.c
> > @@ -30,6 +30,7 @@
> > #include <errno.h>
> > #include <team.h>
> > #include <private/misc.h>
> > +#include <net/ethernet.h>
> >
> > #include "teamd.h"
> > #include "teamd_config.h"
> > @@ -60,6 +61,7 @@ struct lacpdu_info {
> > } __attribute__((__packed__));
> >
> > struct lacpdu {
> > struct ether_header	hdr;
> > uint8_t			subtype;
> > uint8_t			version_number;
> > uint8_t			actor_tlv_type;
> >
> > @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> >    struct lacpdu lacpdu;
> >    struct sockaddr_ll ll_my;
> >    struct sockaddr_ll ll_slow;
> > char *hwaddr;
> > unsigned char hwaddr_len;
> > int err;
> > bool admin_state;
> >
> > @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> >
> > memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> > hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> > hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> > if (hwaddr_len != ETH_ALEN)
> > return 0;
> > lacpdu_init(&lacpdu);
> >  lacpdu.actor = lacp_port->actor;
> >  lacpdu.partner = lacp_port->partner;
> > memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> > memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> > lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> > err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> > 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> > err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> > return err;
> >
> > }
> >
> > @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
> >    	return err;
> >    }
> > err = teamd_packet_sock_open(&lacp_port->sock,
> > 		     tdport->ifindex,
> > 		     htons(ETH_P_SLOW), NULL, NULL);
> > err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> > 			  tdport->ifindex,
> > 			  htons(ETH_P_SLOW), NULL, NULL);
> > if (err)
> > return err;
> >
> > -- 
> > 2.1.4

[Jiri Pirko]

Fri, Apr 15, 2016 at 04:26:21PM CEST, [e-mail] wrote:
> On Fri Apr 15 08:27:02 +0200 2016, Jiri Pirko wrote:
> > Fri, Apr 15, 2016 at 01:14:04AM CEST, [e-mail] wrote:
> > > The team lacp runner sets the mac addresses of all member interfaces
> > >  to be the same value.  However, LACPDUs should carry an interface's
> > >  original (individual) MAC address.
> >
> > ^^
> > Apart these useless spaces, the patch itself looks good to me. 43.4.2.2
> > really says: "Source Address (SA). The SA in LACPDUs carries the
> > individual MAC address associated with the port through which the
> > LACPDU is transmitted." But it does not say anything about original port
> > mac, and the current port mac is the one of the team master. Why is this
> > change needed? Does any aggregator actually care what src lacpdu address
> > is?
>
> The Addressing section (5.2.10 in 802.1ax) explains this a little better:
>    Protocol entities sourcing frames from within the Link Aggregation
>    sublayer (e.g., LACP and the Marker protocol) use the MAC address
>    of the MAC within an underlying port as the source address in
>    frames transmitted through that port.  The MAC Client sees only
>    the Aggregator and not the underlying MACs, and therefore uses
>    the Aggregator's MAC address as the source address in transmitted
>    frames.
>
> I haven't found any implementations that are picky about the source
> address.  However I think this change more closely matches the specified
> behavior.  Also, I find it helpful during debugging since it's immediately
> obvious which interface sent a control packet.

Okay. Fixing the space issue and applying. Thanks.
> Eric
> > > Signed-off-by: Eric Kinzie
> > >
> > > teamd/teamd.h             |  5 +++++
> > > teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
> > > teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
> > > 3 files changed, 58 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/teamd/teamd.h b/teamd/teamd.h
> > > index d8ab6c8..5dbfb9b 100644
> > > --- a/teamd/teamd.h
> > > +++ b/teamd/teamd.h
> > > @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
> > >
> > > int teamd_hash_func_set(struct teamd_context *ctx);
> > >
> > > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > > 		const unsigned short family,
> > > 		const struct sock_fprog *fprog,
> > > 		const struct sock_fprog *alt_fprog);
> > >
> > > int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > >    		   const unsigned short family,
> > >    		   const struct sock_fprog *fprog,
> > > @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> > >    		     size_t expected_len);
> > > int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
> > >    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> > > +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
> > > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> > >    	   struct sockaddr *src_addr, socklen_t addrlen);
> > >
> > > diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> > > index 337cfbf..f32c2ef 100644
> > > --- a/teamd/teamd_common.c
> > > +++ b/teamd/teamd_common.c
> > > @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
> > >    return 0;
> > > }
> > >
> > > -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > > 	   const unsigned short family,
> > > 	   const struct sock_fprog *fprog,
> > > 	   const struct sock_fprog *alt_fprog)
> > >
> > > +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> > > 		const unsigned short family,
> > > 		const struct sock_fprog *fprog,
> > > 		const struct sock_fprog *alt_fprog)
> > >
> > > {
> > >    struct sockaddr_ll ll_my;
> > >    int sock;
> > >    int ret;
> > >    int err;
> > > sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> > > sock = socket(PF_PACKET, type, 0);
> > > if (sock == -1) {
> > > teamd_log_err("Failed to create packet socket.");
> > > return -errno;
> > >
> > > @@ -121,6 +121,15 @@ close_sock:
> > >    return err;
> > > }
> > >
> > > +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> > > 	   const unsigned short family,
> > > 	   const struct sock_fprog *fprog,
> > > 	   const struct sock_fprog *alt_fprog)
> > >
> > > +{
> > > return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> > > 			   fprog, alt_fprog);
> > >
> > > +}
> > >
> > > int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
> > >    		     size_t expected_len)
> > > {
> > > @@ -164,6 +173,29 @@ resend:
> > >    return 0;
> > > }
> > >
> > > +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> > > +{
> > > ssize_t ret;
> > >
> > > +resend:
> > > ret = send(sockfd, buf, len, flags);
> > > if (ret == -1) {
> > > switch(errno) {
> > > case EINTR:
> > > 	goto resend;
> > > case ENETDOWN:
> > > case ENETUNREACH:
> > > case EADDRNOTAVAIL:
> > > case ENXIO:
> > > 	return 0;
> > > default:
> > > 	teamd_log_err("send failed.");
> > > 	return -errno;
> > > }
> > > }
> > > return 0;
> > >
> > > +}
> > >
> > > int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
> > >    	   struct sockaddr *src_addr, socklen_t addrlen)
> > > {
> > > diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> > > index 1af3b17..dc03996 100644
> > > --- a/teamd/teamd_runner_lacp.c
> > > +++ b/teamd/teamd_runner_lacp.c
> > > @@ -30,6 +30,7 @@
> > > #include <errno.h>
> > > #include <team.h>
> > > #include <private/misc.h>
> > > +#include <net/ethernet.h>
> > >
> > > #include "teamd.h"
> > > #include "teamd_config.h"
> > > @@ -60,6 +61,7 @@ struct lacpdu_info {
> > > } __attribute__((__packed__));
> > >
> > > struct lacpdu {
> > > struct ether_header	hdr;
> > > uint8_t			subtype;
> > > uint8_t			version_number;
> > > uint8_t			actor_tlv_type;
> > >
> > > @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> > >    struct lacpdu lacpdu;
> > >    struct sockaddr_ll ll_my;
> > >    struct sockaddr_ll ll_slow;
> > > char *hwaddr;
> > > unsigned char hwaddr_len;
> > > int err;
> > > bool admin_state;
> > >
> > > @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
> > >
> > > memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> > > hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> > > hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> > > if (hwaddr_len != ETH_ALEN)
> > > return 0;
> > > lacpdu_init(&lacpdu);
> > >  lacpdu.actor = lacp_port->actor;
> > >  lacpdu.partner = lacp_port->partner;
> > > memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> > > memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> > > lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> > > err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> > > 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> > > err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> > > return err;
> > >
> > > }
> > >
> > > @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
> > >    	return err;
> > >    }
> > > err = teamd_packet_sock_open(&lacp_port->sock,
> > > 		     tdport->ifindex,
> > > 		     htons(ETH_P_SLOW), NULL, NULL);
> > > err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> > > 			  tdport->ifindex,
> > > 			  htons(ETH_P_SLOW), NULL, NULL);
> > > if (err)
> > > return err;
> > >
> > > -- 
> > > 2.1.4

[Flavio Leitner]

On Thu, Apr 14, 2016 at 04:14:04PM -0700, Eric Kinzie wrote:
> The team lacp runner sets the mac addresses of all member interfaces
>   to be the same value.  However, LACPDUs should carry an interface's
>   original (individual) MAC address.
>
> Signed-off-by: Eric Kinzie

LGTM.
fbl
> teamd/teamd.h             |  5 +++++
>  teamd/teamd_common.c      | 42 +++++++++++++++++++++++++++++++++++++-----
>  teamd/teamd_runner_lacp.c | 21 ++++++++++++++++-----
>  3 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/teamd/teamd.h b/teamd/teamd.h
> index d8ab6c8..5dbfb9b 100644
> --- a/teamd/teamd.h
> +++ b/teamd/teamd.h
> @@ -353,6 +353,10 @@ void teamd_balancer_port_removed(struct teamd_balancer *tb,
>
> int teamd_hash_func_set(struct teamd_context *ctx);
>
> +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> 		const unsigned short family,
> 		const struct sock_fprog *fprog,
> 		const struct sock_fprog *alt_fprog);
>
> int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
>    		   const unsigned short family,
>    		   const struct sock_fprog *fprog,
> @@ -361,6 +365,7 @@ int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
>    		     size_t expected_len);
>  int teamd_sendto(int sockfd, const void *buf, size_t len, int flags,
>    	 const struct sockaddr *dest_addr, socklen_t addrlen);
> +int teamd_send(int sockfd, const void *buf, size_t len, int flags);
>  int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
>    	   struct sockaddr *src_addr, socklen_t addrlen);
>
> diff --git a/teamd/teamd_common.c b/teamd/teamd_common.c
> index 337cfbf..f32c2ef 100644
> --- a/teamd/teamd_common.c
> +++ b/teamd/teamd_common.c
> @@ -81,17 +81,17 @@ static int attach_filter(int sock, const struct sock_fprog *pref_fprog,
>    return 0;
>  }
>
> -int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> 	   const unsigned short family,
> 	   const struct sock_fprog *fprog,
> 	   const struct sock_fprog *alt_fprog)
>
> +int teamd_packet_sock_open_type(int type, int *sock_p, const uint32_t ifindex,
> 		const unsigned short family,
> 		const struct sock_fprog *fprog,
> 		const struct sock_fprog *alt_fprog)
>
> {
>    struct sockaddr_ll ll_my;
>    int sock;
>    int ret;
>    int err;
> sock = socket(PF_PACKET, SOCK_DGRAM, 0);
> sock = socket(PF_PACKET, type, 0);
> if (sock == -1) {
> teamd_log_err("Failed to create packet socket.");
> return -errno;
>
> @@ -121,6 +121,15 @@ close_sock:
>    return err;
>  }
>
> +int teamd_packet_sock_open(int *sock_p, const uint32_t ifindex,
> 	   const unsigned short family,
> 	   const struct sock_fprog *fprog,
> 	   const struct sock_fprog *alt_fprog)
>
> +{
> return teamd_packet_sock_open_type(SOCK_DGRAM, sock_p, ifindex, family,
> 			   fprog, alt_fprog);
>
> +}
>
> int teamd_getsockname_hwaddr(int sock, struct sockaddr_ll *addr,
>    		     size_t expected_len)
>  {
> @@ -164,6 +173,29 @@ resend:
>    return 0;
>  }
>
> +int teamd_send(int sockfd, const void *buf, size_t len, int flags)
> +{
> ssize_t ret;
>
> +resend:
> ret = send(sockfd, buf, len, flags);
> if (ret == -1) {
> switch(errno) {
> case EINTR:
> 	goto resend;
> case ENETDOWN:
> case ENETUNREACH:
> case EADDRNOTAVAIL:
> case ENXIO:
> 	return 0;
> default:
> 	teamd_log_err("send failed.");
> 	return -errno;
> }
> }
> return 0;
>
> +}
>
> int teamd_recvfrom(int sockfd, void *buf, size_t len, int flags,
>    	   struct sockaddr *src_addr, socklen_t addrlen)
>  {
> diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
> index 1af3b17..dc03996 100644
> --- a/teamd/teamd_runner_lacp.c
> +++ b/teamd/teamd_runner_lacp.c
> @@ -30,6 +30,7 @@
>  #include <errno.h>
>  #include <team.h>
>  #include <private/misc.h>
> +#include <net/ethernet.h>
>
> #include "teamd.h"
>  #include "teamd_config.h"
> @@ -60,6 +61,7 @@ struct lacpdu_info {
>  } __attribute__((__packed__));
>
> struct lacpdu {
> struct ether_header	hdr;
> uint8_t			subtype;
> uint8_t			version_number;
> uint8_t			actor_tlv_type;
>
> @@ -1013,6 +1015,8 @@ static int lacpdu_send(struct lacp_port *lacp_port)
>    struct lacpdu lacpdu;
>    struct sockaddr_ll ll_my;
>    struct sockaddr_ll ll_slow;
> char *hwaddr;
> unsigned char hwaddr_len;
> int err;
> bool admin_state;
>
> @@ -1028,12 +1032,19 @@ static int lacpdu_send(struct lacp_port *lacp_port)
>
> memcpy(lacp_port->actor.system, lacp_port->ctx->hwaddr, ETH_ALEN);
> hwaddr = team_get_ifinfo_orig_hwaddr(lacp_port->tdport->team_ifinfo);
> hwaddr_len = team_get_ifinfo_orig_hwaddr_len(lacp_port->tdport->team_ifinfo);
> if (hwaddr_len != ETH_ALEN)
> return 0;
> lacpdu_init(&lacpdu);
>  lacpdu.actor = lacp_port->actor;
>  lacpdu.partner = lacp_port->partner;
> memcpy(lacpdu.hdr.ether_shost, hwaddr, hwaddr_len);
> memcpy(lacpdu.hdr.ether_dhost, ll_slow.sll_addr, ll_slow.sll_halen);
> lacpdu.hdr.ether_type = htons(ETH_P_SLOW);
> err = teamd_sendto(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0,
> 	   (struct sockaddr *) &ll_slow, sizeof(ll_slow));
> err = teamd_send(lacp_port->sock, &lacpdu, sizeof(lacpdu), 0);
> return err;
>
> }
>
> @@ -1201,9 +1212,9 @@ static int lacp_port_added(struct teamd_context *ctx,
>    	return err;
>    }
> err = teamd_packet_sock_open(&lacp_port->sock,
> 		     tdport->ifindex,
> 		     htons(ETH_P_SLOW), NULL, NULL);
> err = teamd_packet_sock_open_type(SOCK_RAW, &lacp_port->sock,
> 			  tdport->ifindex,
> 			  htons(ETH_P_SLOW), NULL, NULL);
> if (err)
> return err;
>
> -- 
> 2.1.4
> _______________________________________________
> libteam mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]