[ovs-dev] [PATCH 1/2] vswitch: Factor out detection of internal interfaces into a new function.

[Ben Pfaff]

Is that for both patches or just 1/2?

Justin Pettit <jpettit at nicira.com> writes:

> Looks good.
>
> --Justin
>
>
> On Oct 2, 2009, at 10:59 AM, Ben Pfaff wrote:
>
>> The following commit needs to use this same logic, so break it out
>> into
>> a function to avoid redundancy.
>> ---
>> vswitchd/bridge.c |   41 ++++++++++++++++++++++++++++++-----------
>> 1 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index b525f3d..2d788ae 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -242,6 +242,7 @@ static void iface_destroy(struct iface *);
>> static struct iface *iface_lookup(const struct bridge *, const char
>> *name);
>> static struct iface *iface_from_dp_ifidx(const struct bridge *,
>>                                          uint16_t dp_ifidx);
>> +static bool iface_is_internal(const struct bridge *, const char
>> *name);
>>
>> /* Hooks into ofproto processing. */
>> static struct ofhooks bridge_ofhooks;
>> @@ -464,18 +465,8 @@ bridge_reconfigure(void)
>>                 bool internal;
>>                 int error;
>>
>> -                /* It's an internal interface if it's marked that
>> way, or if
>> -                 * it's a bonded interface for which we're faking
>> up a network
>> -                 * device. */
>> -                internal = cfg_get_bool(0, "iface.%s.internal",
>> if_name);
>> -                if (cfg_get_bool(0, "bonding.%s.fake-iface",
>> if_name)) {
>> -                    struct port *port = port_lookup(br, if_name);
>> -                    if (port && port->n_ifaces > 1) {
>> -                        internal = true;
>> -                    }
>> -                }
>> -
>>                 /* Add to datapath. */
>> +                internal = iface_is_internal(br, if_name);
>>                 error = dpif_port_add(&br->dpif, if_name,
>> next_port_no++,
>>                                       internal ? ODP_PORT_INTERNAL :
>> 0);
>>                 if (error != EEXIST) {
>> @@ -3116,6 +3107,34 @@ iface_from_dp_ifidx(const struct bridge *br,
>> uint16_t dp_ifidx)
>> {
>>     return port_array_get(&br->ifaces, dp_ifidx);
>> }
>> +
>> +/* Returns true if 'iface' is the name of an "internal" interface
>> on bridge
>> + * 'br', that is, an interface that is entirely simulated within
>> the datapath.
>> + * The local port (ODPP_LOCAL) is always an internal interface.
>> Other local
>> + * interfaces are created by setting "iface.<iface>.internal = true".
>> + *
>> + * In addition, we have a kluge-y feature that creates an internal
>> port with
>> + * the name of a bonded port if "bonding.<bondname>.fake-iface =
>> true" is set.
>> + * This feature needs to go away in the long term.  Until then,
>> this is one
>> + * reason why this function takes a name instead of a struct iface:
>> the fake
>> + * interfaces created this way do not have a struct iface. */
>> +static bool
>> +iface_is_internal(const struct bridge *br, const char *iface)
>> +{
>> +    if (!strcmp(iface, br->name)
>> +        || cfg_get_bool(0, "iface.%s.internal", iface)) {
>> +        return true;
>> +    }
>> +
>> +    if (cfg_get_bool(0, "bonding.%s.fake-iface", iface)) {
>> +        struct port *port = port_lookup(br, iface);
>> +        if (port && port->n_ifaces > 1) {
>> +            return true;
>> +        }
>> +    }
>> +
>> +    return false;
>> +}
>>
>> /* Port mirroring. */
>>
>> --
>> 1.6.3.3
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org