On Tue, Nov 29, 2011 at 07:53:21PM -0800, Ethan Jackson wrote:
> It's a bug if LACP is configured with a system ID of zero. This
> patch assert fails in this case.
Will the change to lacp_configure() have the desired effect? I think
that the common case is that lacp_create() creates an all-zeros
configuration and then lacp_configure() updates it. In such a case
eth_addr_equals() would be true if s->id is zero, since it was already
zero.
Therefore, I think that it would improve the new assertion to move it
outside the "if" statement.
With that change this looks good to me.
It might be nice to add a comment to "struct lacp_settings" that the id
member must be nonzero (and perhaps that struct could use other comments
also).
Thanks,
Ben.
> ---
> lib/lacp.c | 1 +
> vswitchd/bridge.c | 5 +++++
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/lib/lacp.c b/lib/lacp.c
> index 22cba94..9e63bbb 100644
> --- a/lib/lacp.c
> +++ b/lib/lacp.c
> @@ -232,6 +232,7 @@ lacp_configure(struct lacp *lacp, const struct
> lacp_settings *s)
> if (!eth_addr_equals(lacp->sys_id, s->id)
> || lacp->sys_priority != s->priority
> || lacp->heartbeat != s->heartbeat) {
> + assert(!eth_addr_is_zero(s->id));
> memcpy(lacp->sys_id, s->id, ETH_ADDR_LEN);
> lacp->sys_priority = s->priority;
> lacp->heartbeat = s->heartbeat;
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index d140565..b490176 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2658,6 +2658,11 @@ port_configure_lacp(struct port *port, struct
> lacp_settings *s)
> memcpy(s->id, port->bridge->ea, ETH_ADDR_LEN);
> }
>
> + if (eth_addr_is_zero(s->id)) {
> + VLOG_WARN("port %s: Invalid zero LACP system ID.", port->name);
> + return NULL;
> + }
> +
> /* Prefer bondable links if unspecified. */
> priority = atoi(get_port_other_config(port->cfg, "lacp-system-priority",
> "0"));
> --
> 1.7.7
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
