[ovs-dev] [PATCH 2/2] mirroring: Allow learning to be disabled on a VLAN.
Jesse Gross
jesse at nicira.com
Tue Nov 10 21:52:07 UTC 2009
Ben Pfaff wrote:
> Jesse Gross <jesse at nicira.com> writes:
>
>
>> RSPAN does not work properly unless MAC learning for the VLAN is
>> disabled on all switches between the origin and monitoring point.
>> This allows learning to be disabled on a given VLAN so vSwitch can
>> acts as an intermediate switch.
>>
>
> I find double negatives more difficult to understand than single
> ones, so could non_learning_vlan() be changed to
> is_learning_vlan()? It makes sense to keep the array name the
> same, since most of the time it will be a null pointer.
>
That's fine, it makes it a little more readable I guess.
> This could be implemented inside the mac_learning table itself,
> by adding mac_learning functions to enable or disable MAC
> learning on given VLANs. Do you think that is a better way to do
> it? It would push a small amount of complexity out of bridge.c,
> which is already too complex.
>
>
Good idea, there is too much stuff in bridge.c.
>> @@ -1839,6 +1841,13 @@ is_bcast_arp_reply(const flow_t *flow)
>> && eth_addr_is_broadcast(flow->dl_dst));
>> }
>>
>> +static bool
>> +non_learning_vlan(const struct bridge *br, uint16_t vlan)
>> +{
>> + return br->non_learning_vlans &&
>> + bitmap_is_set(br->non_learning_vlans, vlan);
>> +}
>> +
>>
>
> CodingStyle calls for:
> return (br->non_learning_vlans
> && bitmap_is_set(br->non_learning_vlans, vlan));
>
>
OK.
>> + if (rspan_vlans == NULL
>> + ? br->non_learning_vlans != NULL
>> + : br->non_learning_vlans == NULL ||
>> + !bitmap_equal(rspan_vlans, br->non_learning_vlans, 4096)) {
>>
>
> CodingStyle calls for:
>
> if (rspan_vlans == NULL
> ? br->non_learning_vlans != NULL
> : (br->non_learning_vlans == NULL
> || !bitmap_equal(rspan_vlans, br->non_learning_vlans, 4096))) {
>
>
OK.
I pushed it with the above changes.
More information about the dev
mailing list