Thanks for the reviews. I pushed this to master, branch-1.4, branch-1.5.
On Tue, Jan 31, 2012 at 06:16:54PM -0800, Ethan Jackson wrote:
> May as well set the minInteger in the documentation to 15. Otherwise
> looks good.
>
> Ethan
>
> On Tue, Jan 24, 2012 at 13:44, Ben Pfaff <b...@nicira.com> wrote:
> > NICS-11.
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > NEWS | 2 ++
> > lib/learning-switch.c | 6 ++++--
> > lib/mac-learning.c | 41 ++++++++++++++++++++++++++++++++++-------
> > lib/mac-learning.h | 10 +++++++---
> > ofproto/ofproto-dpif.c | 13 +++++++++++--
> > ofproto/ofproto-provider.h | 6 +++++-
> > ofproto/ofproto.c | 10 ++++++++++
> > ofproto/ofproto.h | 3 ++-
> > vswitchd/bridge.c | 17 +++++++++++++++++
> > vswitchd/vswitch.xml | 20 ++++++++++++++++++++
> > 10 files changed, 112 insertions(+), 16 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index d7332f8..14753f1 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -31,6 +31,8 @@ v1.4.0 - xx xxx xxxx
> > broken device drivers are no longer in widespread use, we will
> > delete this feature.) See ovs-vswitchd.conf.db(5) for more
> > information.
> > + - The default MAC learning timeout has been increased from 60 seconds
> > + to 300 seconds. The MAC learning timeout is now configurable.
> > - OpenFlow:
> > - Added ability to match on IPv6 flow label through NXM.
> > - Added ability to match on ECN bits in IPv4 and IPv6 through NXM.
> > diff --git a/lib/learning-switch.c b/lib/learning-switch.c
> > index 3bcb961..60cf731 100644
> > --- a/lib/learning-switch.c
> > +++ b/lib/learning-switch.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -97,7 +97,9 @@ lswitch_create(struct rconn *rconn, const struct
> > lswitch_config *cfg)
> > sw->max_idle = cfg->max_idle;
> > sw->datapath_id = 0;
> > sw->last_features_request = time_now() - 1;
> > - sw->ml = cfg->mode == LSW_LEARN ? mac_learning_create() : NULL;
> > + sw->ml = (cfg->mode == LSW_LEARN
> > + ? mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME)
> > + : NULL);
> > sw->action_normal = cfg->mode == LSW_NORMAL;
> >
> > flow_wildcards_init_exact(&sw->wc);
> > diff --git a/lib/mac-learning.c b/lib/mac-learning.c
> > index efd1dd4..836bcd4 100644
> > --- a/lib/mac-learning.c
> > +++ b/lib/mac-learning.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -37,12 +37,12 @@ VLOG_DEFINE_THIS_MODULE(mac_learning);
> > COVERAGE_DEFINE(mac_learning_learned);
> > COVERAGE_DEFINE(mac_learning_expired);
> >
> > -/* Returns the number of seconds since 'e' was last learned. */
> > +/* Returns the number of seconds since 'e' (within 'ml') was last learned.
> > */
> > int
> > -mac_entry_age(const struct mac_entry *e)
> > +mac_entry_age(const struct mac_learning *ml, const struct mac_entry *e)
> > {
> > time_t remaining = e->expires - time_now();
> > - return MAC_ENTRY_IDLE_TIME - remaining;
> > + return ml->idle_time - remaining;
> > }
> >
> > static uint32_t
> > @@ -98,9 +98,18 @@ get_lru(struct mac_learning *ml, struct mac_entry **e)
> > }
> > }
> >
> > -/* Creates and returns a new MAC learning table. */
> > +static unsigned int
> > +normalize_idle_time(unsigned int idle_time)
> > +{
> > + return (idle_time < 15 ? 15
> > + : idle_time > 3600 ? 3600
> > + : idle_time);
> > +}
> > +
> > +/* Creates and returns a new MAC learning table with an initial MAC aging
> > + * timeout of 'idle_time' seconds. */
> > struct mac_learning *
> > -mac_learning_create(void)
> > +mac_learning_create(unsigned int idle_time)
> > {
> > struct mac_learning *ml;
> >
> > @@ -109,6 +118,7 @@ mac_learning_create(void)
> > hmap_init(&ml->table);
> > ml->secret = random_uint32();
> > ml->flood_vlans = NULL;
> > + ml->idle_time = normalize_idle_time(idle_time);
> > return ml;
> > }
> >
> > @@ -146,6 +156,23 @@ mac_learning_set_flood_vlans(struct mac_learning *ml,
> > }
> > }
> >
> > +/* Changes the MAC aging timeout of 'ml' to 'idle_time' seconds. */
> > +void
> > +mac_learning_set_idle_time(struct mac_learning *ml, unsigned int idle_time)
> > +{
> > + idle_time = normalize_idle_time(idle_time);
> > + if (idle_time != ml->idle_time) {
> > + struct mac_entry *e;
> > + int delta;
> > +
> > + delta = (int) idle_time - (int) ml->idle_time;
> > + LIST_FOR_EACH (e, lru_node, &ml->lrus) {
> > + e->expires += delta;
> > + }
> > + ml->idle_time = idle_time;
> > + }
> > +}
> > +
> > static bool
> > is_learning_vlan(const struct mac_learning *ml, uint16_t vlan)
> > {
> > @@ -199,7 +226,7 @@ mac_learning_insert(struct mac_learning *ml,
> >
> > /* Mark 'e' as recently used. */
> > list_push_back(&ml->lrus, &e->lru_node);
> > - e->expires = time_now() + MAC_ENTRY_IDLE_TIME;
> > + e->expires = time_now() + ml->idle_time;
> >
> > return e;
> > }
> > diff --git a/lib/mac-learning.h b/lib/mac-learning.h
> > index 4bcf221..8d0ac62 100644
> > --- a/lib/mac-learning.h
> > +++ b/lib/mac-learning.h
> > @@ -24,10 +24,12 @@
> > #include "tag.h"
> > #include "timeval.h"
> >
> > +struct mac_learning;
> > +
> > #define MAC_MAX 2048
> >
> > /* Time, in seconds, before expiring a mac_entry due to inactivity. */
> > -#define MAC_ENTRY_IDLE_TIME 300
> > +#define MAC_ENTRY_DEFAULT_IDLE_TIME 300
> >
> > /* Time, in seconds, to lock an entry updated by a gratuitous ARP to avoid
> > * relearning based on a reflection from a bond slave. */
> > @@ -50,7 +52,7 @@ struct mac_entry {
> > } port;
> > };
> >
> > -int mac_entry_age(const struct mac_entry *);
> > +int mac_entry_age(const struct mac_learning *, const struct mac_entry *);
> >
> > /* Returns true if mac_learning_insert() just created 'mac' and the caller
> > has
> > * not yet properly initialized it. */
> > @@ -80,10 +82,11 @@ struct mac_learning {
> > front, most recently used at the back. */
> > uint32_t secret; /* Secret for randomizing hash table. */
> > unsigned long *flood_vlans; /* Bitmap of learning disabled VLANs. */
> > + unsigned int idle_time; /* Max age before deleting an entry. */
> > };
> >
> > /* Basics. */
> > -struct mac_learning *mac_learning_create(void);
> > +struct mac_learning *mac_learning_create(unsigned int idle_time);
> > void mac_learning_destroy(struct mac_learning *);
> >
> > void mac_learning_run(struct mac_learning *, struct tag_set *);
> > @@ -92,6 +95,7 @@ void mac_learning_wait(struct mac_learning *);
> > /* Configuration. */
> > bool mac_learning_set_flood_vlans(struct mac_learning *,
> > const unsigned long *bitmap);
> > +void mac_learning_set_idle_time(struct mac_learning *, unsigned int
> > idle_time);
> >
> > /* Learning. */
> > bool mac_learning_may_learn(const struct mac_learning *,
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 471ba64..e3edec0 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -649,7 +649,7 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
> > ofproto->sflow = NULL;
> > ofproto->stp = NULL;
> > hmap_init(&ofproto->bundles);
> > - ofproto->ml = mac_learning_create();
> > + ofproto->ml = mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME);
> > for (i = 0; i < MAX_MIRRORS; i++) {
> > ofproto->mirrors[i] = NULL;
> > }
> > @@ -2147,6 +2147,13 @@ forward_bpdu_changed(struct ofproto *ofproto_)
> > /* Revalidate cached flows whenever forward_bpdu option changes. */
> > ofproto->need_revalidate = true;
> > }
> > +
> > +static void
> > +set_mac_idle_time(struct ofproto *ofproto_, unsigned int idle_time)
> > +{
> > + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> > + mac_learning_set_idle_time(ofproto->ml, idle_time);
> > +}
> >
> > /* Ports. */
> >
> > @@ -5847,7 +5854,8 @@ ofproto_unixctl_fdb_show(struct unixctl_conn *conn,
> > int argc OVS_UNUSED,
> > struct ofbundle *bundle = e->port.p;
> > ds_put_format(&ds, "%5d %4d "ETH_ADDR_FMT" %3d\n",
> > ofbundle_get_a_port(bundle)->odp_port,
> > - e->vlan, ETH_ADDR_ARGS(e->mac), mac_entry_age(e));
> > + e->vlan, ETH_ADDR_ARGS(e->mac),
> > + mac_entry_age(ofproto->ml, e));
> > }
> > unixctl_command_reply(conn, 200, ds_cstr(&ds));
> > ds_destroy(&ds);
> > @@ -6336,5 +6344,6 @@ const struct ofproto_class ofproto_dpif_class = {
> > set_flood_vlans,
> > is_mirror_output_bundle,
> > forward_bpdu_changed,
> > + set_mac_idle_time,
> > set_realdev,
> > };
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index b995831..f1a7e9a 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -1062,6 +1062,10 @@ struct ofproto_class {
> > * will be invoked. */
> > void (*forward_bpdu_changed)(struct ofproto *ofproto);
> >
> > + /* Sets the MAC aging timeout for the OFPP_NORMAL action to
> > 'idle_time',
> > + * in seconds. */
> > + void (*set_mac_idle_time)(struct ofproto *ofproto, unsigned int
> > idle_time);
> > +
> > /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
> > *
> > * This is deprecated. It is only for compatibility with broken device
> > drivers
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 0504026..9913a57 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -447,6 +447,16 @@ ofproto_set_forward_bpdu(struct ofproto *ofproto, bool
> > forward_bpdu)
> > }
> > }
> >
> > +/* Sets the MAC aging timeout for the OFPP_NORMAL action on 'ofproto' to
> > + * 'idle_time', in seconds. */
> > +void
> > +ofproto_set_mac_idle_time(struct ofproto *ofproto, unsigned idle_time)
> > +{
> > + if (ofproto->ofproto_class->set_mac_idle_time) {
> > + ofproto->ofproto_class->set_mac_idle_time(ofproto, idle_time);
> > + }
> > +}
> > +
> > void
> > ofproto_set_desc(struct ofproto *p,
> > const char *mfr_desc, const char *hw_desc,
> > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> > index 2d47878..24efa15 100644
> > --- a/ofproto/ofproto.h
> > +++ b/ofproto/ofproto.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -205,6 +205,7 @@ void ofproto_set_extra_in_band_remotes(struct ofproto *,
> > void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
> > void ofproto_set_flow_eviction_threshold(struct ofproto *, unsigned
> > threshold);
> > void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
> > +void ofproto_set_mac_idle_time(struct ofproto *, unsigned idle_time);
> > void ofproto_set_desc(struct ofproto *,
> > const char *mfr_desc, const char *hw_desc,
> > const char *sw_desc, const char *serial_desc,
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index c40bcb5..625b96e 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -32,6 +32,7 @@
> > #include "jsonrpc.h"
> > #include "lacp.h"
> > #include "list.h"
> > +#include "mac-learning.h"
> > #include "netdev.h"
> > #include "ofp-print.h"
> > #include "ofpbuf.h"
> > @@ -156,6 +157,7 @@ static void bridge_configure_datapath_id(struct bridge
> > *);
> > static void bridge_configure_flow_eviction_threshold(struct bridge *);
> > static void bridge_configure_netflow(struct bridge *);
> > static void bridge_configure_forward_bpdu(struct bridge *);
> > +static void bridge_configure_mac_idle_time(struct bridge *);
> > static void bridge_configure_sflow(struct bridge *, int
> > *sflow_bridge_number);
> > static void bridge_configure_stp(struct bridge *);
> > static void bridge_configure_remotes(struct bridge *,
> > @@ -465,6 +467,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> > *ovs_cfg)
> > bridge_configure_mirrors(br);
> > bridge_configure_flow_eviction_threshold(br);
> > bridge_configure_forward_bpdu(br);
> > + bridge_configure_mac_idle_time(br);
> > bridge_configure_remotes(br, managers, n_managers);
> > bridge_configure_netflow(br);
> > bridge_configure_sflow(br, &sflow_bridge_number);
> > @@ -1271,6 +1274,20 @@ bridge_configure_forward_bpdu(struct bridge *br)
> > ofproto_set_forward_bpdu(br->ofproto, forward_bpdu);
> > }
> >
> > +/* Set MAC aging time for 'br'. */
> > +static void
> > +bridge_configure_mac_idle_time(struct bridge *br)
> > +{
> > + const char *idle_time_str;
> > + int idle_time;
> > +
> > + idle_time_str = bridge_get_other_config(br->cfg, "mac-aging-time");
> > + idle_time = (idle_time_str && atoi(idle_time_str)
> > + ? atoi(idle_time_str)
> > + : MAC_ENTRY_DEFAULT_IDLE_TIME);
> > + ofproto_set_mac_idle_time(br->ofproto, idle_time);
> > +}
> > +
> > static void
> > bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
> > struct iface **hw_addr_iface)
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 8c506e4..da5508e 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -553,6 +553,26 @@
> > should be enabled. Default is disabled, set to
> > <code>true</code> to enable.
> > </column>
> > +
> > + <column name="other_config" key="mac-aging-time"
> > + type='{"type": "integer", "minInteger": 1}'>
> > + <p>
> > + The maximum number of seconds to retain a MAC learning entry for
> > + which no packets have been seen. The default is currently 300
> > + seconds (5 minutes). The value, if specified, is forced into a
> > + reasonable range, currently 15 to 3600 seconds.
> > + </p>
> > +
> > + <p>
> > + A short MAC aging time allows a network to more quickly detect
> > that a
> > + host is no longer connected to a switch port. However, it also
> > makes
> > + it more likely that packets will be flooded unnecessarily, when
> > they
> > + are addressed to a connected host that rarely transmits packets.
> > To
> > + reduce the incidence of unnecessary flooding, use a MAC aging
> > time
> > + longer than the maximum interval at which a host will ordinarily
> > + transmit packets.
> > + </p>
> > + </column>
> > </group>
> >
> > <group title="Bridge Status">
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
