[ovs-discuss] [PATCH] vswitchd: Mirror nothing, not everything, if mirror ports don't exist.

Justin Pettit jpettit at nicira.com
Wed Aug 26 21:11:39 UTC 2009


Makes sense.

--Justin

(I'm hoping to keep all code reviews down to two words today.)


On Aug 24, 2009, at 11:07 AM, Ben Pfaff wrote:

> If all of the ports specified as mirror selection criteria actually  
> do not
> exist, then until now the bridge would mirror all incoming packets (on
> specified VLAN(s), if any).  This matches the behavior that occurs  
> if no
> mirror selection ports were specified at all, and so it makes a  
> certain
> amount of logical sense.
>
> But it is far more likely that the user simply misspelled a port  
> name, or
> specified the name of a port that does not always exist.  In fact we  
> have
> seen this behavior in practice when the controller has not caught up  
> to
> the switch's current configuration.  So this commit changes the  
> bridge to
> instead disable a mirror if ports are specified and none of those  
> ports
> exist.
>
> Bug #1904.
> ---
> vswitchd/bridge.c |    9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 3ffa671..6ee2ef5 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -3319,6 +3319,7 @@ mirror_reconfigure_one(struct mirror *m)
>     int *vlans;
>     size_t i;
>     bool mirror_all_ports;
> +    bool any_ports_specified;
>
>     /* Get output port. */
>     out_port_name = cfg_get_key(0, "mirror.%s.%s.output.port",
> @@ -3357,11 +3358,18 @@ mirror_reconfigure_one(struct mirror *m)
>     cfg_get_all_keys(&src_ports, "%s.select.src-port", pfx);
>     cfg_get_all_keys(&dst_ports, "%s.select.dst-port", pfx);
>     cfg_get_all_keys(&ports, "%s.select.port", pfx);
> +    any_ports_specified = src_ports.n || dst_ports.n || ports.n;
>     svec_append(&src_ports, &ports);
>     svec_append(&dst_ports, &ports);
>     svec_destroy(&ports);
>     prune_ports(m, &src_ports);
>     prune_ports(m, &dst_ports);
> +    if (any_ports_specified && !src_ports.n && !dst_ports.n) {
> +        VLOG_ERR("%s: none of the specified ports exist; "
> +                 "disabling port mirror %s", pfx, pfx);
> +        mirror_destroy(m);
> +        goto exit;
> +    }
>
>     /* Get all the vlans, and drop duplicate and invalid vlans. */
>     svec_init(&vlan_strings);
> @@ -3413,6 +3421,7 @@ mirror_reconfigure_one(struct mirror *m)
>     }
>
>     /* Clean up. */
> +exit:
>     svec_destroy(&src_ports);
>     svec_destroy(&dst_ports);
>     free(pfx);
> -- 
> 1.6.3.3
>
>
> _______________________________________________
> discuss mailing list
> discuss at openvswitch.org
> http://openvswitch.org/mailman/listinfo/discuss_openvswitch.org





More information about the discuss mailing list