On Sat, Jul 30, 2011 at 01:41:04AM +0200, Philippe Jung wrote:
> This patchs adds the PVID (Port VLAN ID) tagging feature to the
> switch ports:
> - When an untagged packet is received by a switch port from an
> external device,
>   the packet is tagged with PVID;
> - Tagged packets received by a switch port from an external device are
>   unchanged;
> - When a tagged packet is sent by a switch port to an external
> device, if the
>   tag is egal to the PVID the packet may optionnaly be untagged
> - In all other cases, tagged packet sent by a switch port to an
> external device
>   are unchanged

Thank you for the patch.  After looking around the net for a while, I
can see that some network equipment vendors also support this feature.
It is relatively simple to implement, as I can see from your patch,
potentially useful to users, and not difficult to understand or
maintain, so I see no reason not to implement it in Open vSwitch also
given that you are willing to work on it.

Your implementation is against Open vSwitch 1.1.1.  To commit it, you
will have to rewrite it against the Git "master" branch.  A lot of
code has moved around in the meantime, so this will require a little
bit of work.

The patch was sent word-wrapped, so it can't be applied directly.

The description of the patch is very good, more than sufficient for a
commit log entry.  However the patch doesn't update the user
documentation for how to configure the vswitch, in
vswitchd/vswitch.xml.  Please add that in the next version of the
patch.

In vswitch/vswitch.ovsschema, it would be better to specify the pvid
column as optional and limit its value to the range 0...4095.  That's
the same as the "tag" column, so you can reuse the JSON from there.

I'd actually be inclined to, instead of adding two new columns, add a
single new "vlan_mode" column with an enumerated choice of values:

        empty: Same behavior as currently, for compatibility.

        "trunk": This port is a trunk.  The "tag" column is ignored.

        "access": This port is an access port.  The "trunks" column is
        ignored.

        "pvid": The "tag" and "trunks" columns are both used.  This
        acts as a tagged PVID.

        "untagged-pvid": Same as "pvid" but untagged.

Does that make sense?

Thanks,

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to