On Fri Apr 15 08:27:02 +0200 2016, Jiri Pirko wrote:
Fri, Apr 15, 2016 at 01:14:04AM CEST, ehkinzie(a)gmail.com 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 <ehkinzie(a)gmail.com>
> >---
> > 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
> >