Add option to disable priority tagged packets (VID=0)

This patch adds an option to disable the creation of tagged priority packets with VID=0. This is for the feature_vlan_tagging openvpn-testing head.

I tested the vlan feature and it works fine for me (no dhcp tested). Therefore I bridged my eth0 (LAN) and tap0 (OpenVPN) but as my switch flags arp replys with priority, the client ended up with 802.1q tagged (VID=0) priority packets. These were not expected on the client (Ubuntu 10.04 lts) and I found a linux kernel discussion from summer 2010 about supporting VID=0 priority packets, so I expect more linux clients (windows untested) to not support this kind of packets.
This option prevents the creating of these packets by ignoring the priority information.

This patch has already been tested and works fine for me.

---
 multi.c   |    2 +-
 options.c |    9 +++++++++
 options.h |    1 +
 3 files changed, 11 insertions(+), 1 deletions(-)

index b77791a..d09fa68 100644
@@ -2246,7 +2246,7 @@ remove_vlan_tag (const struct context *c, struct buffer *buf)
       return c->options.vlan_pvid;
     }
 
-  if (pcp == 0)
+  if (pcp == 0 || c->options.vlan_disable_priority)
     {
       /* VLAN-tagged without priority information. */
 

index 827b9f0..fe0eac6 100644
@@ -763,6 +763,7 @@ init_options (struct options *o, const bool init_gc)
 #ifdef ENABLE_VLAN_TAGGING
   o->vlan_accept = VAF_ALL;
   o->vlan_pvid = 1;
+  o->vlan_disable_priority = false;
 #endif
 }
 

@@ -1037,6 +1038,7 @@ show_p2mp_parms (const struct options *o)
   SHOW_BOOL (vlan_tagging);
   msg (D_SHOW_PARMS, "  vlan_accept = %s", print_vlan_accept (o->vlan_accept));
   SHOW_INT (vlan_pvid);
+  SHOW_BOOL (vlan_disable_priority);
 #endif
 #endif /* P2MP_SERVER */
 

@@ -1783,6 +1785,8 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	    msg (M_USAGE, "--vlan-accept requires --vlan-tagging");
 	  if (options->vlan_pvid != defaults.vlan_pvid)
 	    msg (M_USAGE, "--vlan-pvid requires --vlan-tagging");
+	  if (options->vlan_disable_priority != defaults.vlan_disable_priority)
+	    msg (M_USAGE, "--vlan_disable_priority requires --vlan-tagging");
 	}
 #endif
     }

@@ -5782,6 +5786,11 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->vlan_tagging = true;
     }
+  else if (streq (p[0], "vlan-disable-priority"))
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->vlan_disable_priority = true;
+    }
   else if (streq (p[0], "vlan-accept") && p[1])
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);

index a278561..5444e37 100644
@@ -523,6 +523,7 @@ struct options
   bool vlan_tagging;
   enum vlan_acceptable_frames vlan_accept;
   uint16_t vlan_pvid;
+  bool vlan_disable_priority;
 #endif
 };
 

1.7.3.4

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Adding Fabian to Cc, he knows more about the VLAN code.

With this feedback, I'm willing merge in the feat_vlan_tagging branch into master. I'm not sure if this should happen now in the v2.3 time frame (which is getting more and more ready for alpha/beta releases), or if we will take it in the next round with v2.4. But I will bring that up for discussion soonish.

kind regards,

David Sommerseth

Cloud Services Checklist: Pricing and Packaging Optimization

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7gjjgACgkQDC186MBRfrqtsACgm8JiXbRQPTicmDjxL0WxDfGC KQkAoJu4gKtBtu6esT582SCBTD0z5J15
=q6lu
-----END PGP SIGNATURE-----

Missing usage, man.

Hi Michael, Hi David, Hi Alon,

Great to know that other people are using the patch-set too. Your patch is quite similar to a patch [0] we've been using for a year, but which never got pushed into the upstream branch because it was missing documentation. So I fully agree that it's necessary and I'm very sorry that you had to duplicate the work just because I got massively side-tracked.

We're currently using the VLAN patches at three locations at our University and all of them have the strip-priority option activated, so I'd say that it should be the default mode.

I'd even go so far as to say that this doesn't even need to be configurable. Vlan-tagged packets should never remain tagged (neither vlan nor priority tagged) when forwarded to an untagged network. Only when the packet is priority-tagged without vlan-tagging can we assume that the sender knew full well that some clients might not know what to do with them. The reworked patch [1] does precisely that, but it's currently only compile tested, because I ran out of time again. (The reworked patch is part of the VLAN patch-set [2] that was rebased to master.)

To fully test the rebased patch-set in our setups I will also need to port another patch-set (regarding deferred client-connect scripts) ... which is part of the reason why I ran out of time again.

As I've been getting some friendly pressure from the people using the VLAN patch-set (and the other patch-set) at our Uni, this hopefully provides enough motivation to get me working on this stuff again... :)

Thanks for the nudge.

What I was meaning to ask: Did you guys come up with some kind of test framework in the last year? I was looking at creating something like that for the VLAN patchset, but it was far too time intensive for my extremely limited OpenVPN time budget ...

So would manual testing reports suffice? And Michael, would you be willing to review and test the full, rebased patch-set (as soon as I'm confident again that it actually works)?

Am 08.12.2011 11:27, schrieb Alon Bar-Lev:

There are probably not *that* many users, agreed. It's probably more of a "large enterprise" feature, as only people with VLAN tagged networks would be interested. For example, our university is very fond of layer 2 networking and therefore uses VLAN tagging extensively. OpenVPN with VLAN tagging provides a perfect match, allowing a single VPN entry point for access to all the different layer2 networks. I imagine that other large organisations could make use of this feature too.

So the number of deployments using the feature will probably never be high. But the number of users depending on it, might not be as insignificant. The two main deployments at our Uni (currently both in public beta) currently have about 100 unique users with several thousand potential users.

Cheers
Fabian

0:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=commit;h=a642faabff003e8f199341b8af407e91c66e568e
1:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=commit;h=f420df8521b5821fd7ddbbd742b515ecdac9c6c1
2:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=shortlog;h=refs/heads/feat_vlan

I fail to understand why this is relevant as far as usage (openvpn --help) and manual (man openvpn) to document this.

Alon.

Hi Alon,

Am 08.12.2011 21:17, schrieb Alon Bar-Lev:

Ah. Sorry for the misunderstanding.

Are you referring to the specific patch by Michael? I agree that it would have needed more documentation before being acceptable for inclusion. Or are you referring to the VLAN patch-set in general? (BTW, the latest version adds a few additional clarifications to the openvpn man page [0].)

Cheers
Fabian

0:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=commit;h=5c5cce29f2230a8fdaf7b135d2e3255f54af1395

Hi,

the usage here is with the FeM e.V. [1] WLAN project in order to reuse the existing vlans and ip assignments on them.

Testing should be no problem. I currently use a gentoo ebuild that points to the git feature branch + patches so that part should be easy. Further, the openvpn server here is currently testing only (until the wlan gets ready) so I'm free to break it again ;). But I might need some time to put it into the schedule.

Regards,
M. Braun
--
[1] http://www.fem.tu-ilmenau.de/

Hi,

There's the t_client.sh/t_client.rc frame work, which works quite well to run well-defined

- invoke openvpn (-client) like *this*
- then test that ping to *that* list of addresses works

test sets. Creating the tests can be a bit of a nuisance, but after that it really eases automated testing :-) (and of course you need a known-working reference server).

Automated testing of the server end is a bit more tricky, and thus not yet done.

gert

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...