Skip to contentSkip to conversation

vlan: Add global, per-client 802.1q-based options

PATCH In review Mike Auty 3 participants 4 messages Apr 3, 2016

lore
Share & tools
lore

Mike Auty

author
#0 lore raw

PATCH 3/7 vlan: Add global, per-client 802.1q-based options

This patch add the new global "--vlan-tagging" boolean switch. This specifies whether openvpn should handle 802.1q tagged packets in any way.

This patch also adds the new global '--vlan-accept tagged|untagged|all' which specifies the behaviour of the tap device with regards to 802.1q vlan tagged packets.

This patch also adds the new "--vlan-pvid" integer option. The option is valid in server mode and can be set in a client context (e.g. from the client-connect hook). It defaults to 1.

The value indicates which VID (VLAN identifier) to associate with a client or (in global context) with the tap device.
It also serves as the Port VLAN Identifier, i.e. the VID to use for incoming and outgoing untagged or priority-tagged frames.

The client will only receive packets which belong to the same VLAN.

These options have no immediate effect yet, but will be used by later patches.

Patch authored by Fabian Knittel <[email protected]...>.

---
 src/openvpn/options.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
src/openvpn/options.c +85 -0
index 02def3a..7616f23 100644
@@ -477,6 +477,11 @@ static const char usage_message[] =
   "                  sessions to a web server at host:port.  dir specifies an\n"
   "                  optional directory to write origin IP:port data.\n"
 #endif
+#ifdef ENABLE_VLAN_TAGGING
+  "--vlan-tagging  : Enable 802.1Q-based VLAN tagging.\n"
+  "--vlan-accept tagged|untagged|all : Set VLAN tagging mode. Default is 'all'.\n"
+  "--vlan-pvid v   : Sets the Port VLAN Identifier. Defaults to 1.\n"
+#endif
 #endif
   "\n"
   "Client options (when connecting to a multi-client server):\n"
@@ -852,6 +857,10 @@ init_options (struct options *o, const bool init_gc)
 #ifdef ENABLE_PKCS11
   o->pkcs11_pin_cache_period = -1;
 #endif			/* ENABLE_PKCS11 */
+#ifdef ENABLE_VLAN_TAGGING
+  o->vlan_accept = VAF_ALL;
+  o->vlan_pvid = 1;
+#endif
 
 /* tmp is only used in P2MP server context */
 #if P2MP_SERVER
@@ -1160,6 +1169,23 @@ dhcp_option_address_parse (const char *name, const char *parm, in_addr_t *array,
 
 #endif
 
+#ifdef ENABLE_VLAN_TAGGING
+static const char *
+print_vlan_accept (enum vlan_acceptable_frames mode)
+{
+  switch (mode)
+   {
+    case VAF_ONLY_VLAN_TAGGED:
+      return "tagged";
+    case VAF_ONLY_UNTAGGED_OR_PRIORITY:
+      return "untagged";
+    case VAF_ALL:
+      return "all";
+   }
+  return NULL;
+}
+#endif
+
 #if P2MP
 
 #ifndef ENABLE_SMALL
@@ -1225,6 +1251,11 @@ show_p2mp_parms (const struct options *o)
   SHOW_STR (port_share_host);
   SHOW_STR (port_share_port);
 #endif
+#ifdef ENABLE_VLAN_TAGGING
+  SHOW_BOOL (vlan_tagging);
+  msg (D_SHOW_PARMS, "  vlan_accept = %s", print_vlan_accept (o->vlan_accept));
+  SHOW_INT (vlan_pvid);
+#endif
 #endif /* P2MP_SERVER */
 
   SHOW_BOOL (client);
@@ -2076,6 +2107,17 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
 	  if ((options->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) && !ccnr)
 	    msg (M_USAGE, "--auth-user-pass-optional %s", postfix);
 	}
+#ifdef ENABLE_VLAN_TAGGING
+      if (options->vlan_tagging && dev != DEV_TYPE_TAP)
+	msg (M_USAGE, "--vlan-tagging must be used with --dev tap");
+      if (!options->vlan_tagging)
+	{
+	  if (options->vlan_accept != defaults.vlan_accept)
+	    msg (M_USAGE, "--vlan-accept requires --vlan-tagging");
+	  if (options->vlan_pvid != defaults.vlan_pvid)
+	    msg (M_USAGE, "--vlan-pvid requires --vlan-tagging");
+	}
+#endif
     }
   else
     {
@@ -2122,6 +2164,10 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
       if (options->port_share_host || options->port_share_port)
 	msg (M_USAGE, "--port-share requires TCP server mode (--mode server --proto tcp-server)");
 #endif
+#ifdef ENABLE_VLAN_TAGGING
+      if (options->vlan_tagging)
+	msg (M_USAGE, "--vlan-tagging requires --mode server");
+#endif
 
       if (options->stale_routes_check_interval)
         msg (M_USAGE, "--stale-routes-check requires --mode server");
@@ -7178,6 +7224,45 @@ add_option (struct options *options,
       options->keying_material_exporter_length = ekm_length;
     }
 #endif
+#ifdef ENABLE_VLAN_TAGGING
+  else if (streq (p[0], "vlan-tagging"))
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->vlan_tagging = true;
+    }
+  else if (streq (p[0], "vlan-accept") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      if (streq (p[1], "tagged"))
+	{
+	  options->vlan_accept = VAF_ONLY_VLAN_TAGGED;
+	}
+      else if (streq (p[1], "untagged"))
+	{
+	  options->vlan_accept = VAF_ONLY_UNTAGGED_OR_PRIORITY;
+	}
+      else if (streq (p[1], "all"))
+	{
+	  options->vlan_accept = VAF_ALL;
+	}
+      else
+	{
+	  msg (msglevel, "--vlan-accept must be 'tagged', 'untagged' or 'all'");
+	  goto err;
+	}
+    }
+  else if (streq (p[0], "vlan-pvid") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_INSTANCE);
+      options->vlan_pvid = positive_atoi (p[1]);
+      if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID ||
+	  options->vlan_pvid > OPENVPN_8021Q_MAX_VID)
+	{
+	  msg (msglevel, "the parameter of --vlan-pvid parameters must be >= %u and <= %u", OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID);
+	  goto err;
+	}
+    }
+#endif
   else
     {
       int i;
Signature · 1 line
2.7.1
#1 lore raw
Quoted 9 lines · Mike Auty

This patch add the new global "--vlan-tagging" boolean switch. This specifies whether openvpn should handle 802.1q tagged packets in any way.

This patch also adds the new global '--vlan-accept tagged|untagged|all' which specifies the behaviour of the tap device with regards to 802.1q vlan tagged packets.

This patch also adds the new "--vlan-pvid" integer option.

<snip>

Quoted 36 lines
src/openvpn/options.c +32 -0
@@ -7178,6 +7224,45 @@ add_option (struct options *options,
       options->keying_material_exporter_length = ekm_length;
     }
 #endif
+#ifdef ENABLE_VLAN_TAGGING
+  else if (streq (p[0], "vlan-tagging"))
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->vlan_tagging = true;
+    }
+  else if (streq (p[0], "vlan-accept") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      if (streq (p[1], "tagged"))
+       {
+         options->vlan_accept = VAF_ONLY_VLAN_TAGGED;
+       }
+      else if (streq (p[1], "untagged"))
+       {
+         options->vlan_accept = VAF_ONLY_UNTAGGED_OR_PRIORITY;
+       }
+      else if (streq (p[1], "all"))
+       {
+         options->vlan_accept = VAF_ALL;
+       }
+      else
+       {
+         msg (msglevel, "--vlan-accept must be 'tagged', 'untagged' or 'all'");
+         goto err;
+       }
+    }
+  else if (streq (p[0], "vlan-pvid") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_INSTANCE);
+      options->vlan_pvid = positive_atoi (p[1]);
+      if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID ||

src/openvpn/options.c add_option (struct options *options, · line 7268

Quoted 10 lines
src/openvpn/options.c +7 -0
+         options->vlan_pvid > OPENVPN_8021Q_MAX_VID)
+       {
+         msg (msglevel, "the parameter of --vlan-pvid parameters must be >= %u and <= %u", OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID);
+         goto err;
+       }
+    }
+#endif
   else
     {
       int i;

NAK. This should include testing for extra parameters supplied in error with the options, as the rest of the option-handling code does. Something like the following (for each of the three new options):

+ else if (streq (p[0], "vlan-tagging") && !p[1])

author
#2 lore raw
Quoted 7 lines · Jonathan K. Bullard

NAK. This should include testing for extra parameters supplied in error with the options, as the rest of the option-handling code does. Something like the following (for each of the three new options):

+ else if (streq (p[0], "vlan-tagging") && !p[1])

Thanks for reviewing the patches! 5:)

That should be reasonable to add in, I'll try and do that in the next couple of days (and try and change round my git repo so that I can keep the patch versions in separate branches).

Could you let me know if I should be resending all the patches for the next version, or just the ones that have changed?

Mike 5:)

#3 lore raw
Quoted 9 lines · Mike Auty
Quoted 7 lines · Jonathan K. Bullard

NAK. This should include testing for extra parameters supplied in error with the options, as the rest of the option-handling code does. Something like the following (for each of the three new options):

+ else if (streq (p[0], "vlan-tagging") && !p[1])

Quoted 8 lines

Thanks for reviewing the patches! 5:)

That should be reasonable to add in, I'll try and do that in the next couple of days (and try and change round my git repo so that I can keep the patch versions in separate branches).

Could you let me know if I should be resending all the patches for the next version, or just the ones that have changed?

As long as it is a change only affecting a single patch, then resending an updated version of that patch is generally fine.

When updating patches, please ensure that the subject line states for example "PATCH v2". That is easily done by adding --subject-prefix="PATCH v2" to the git command lines creating the patch.

Second detail is to add a "revision history" in the git commit log. See commit a24dd2e31f196c76 as an example.

Signature · 3 lines
kind regards,

David Sommerseth