diff mbox series

[2/3] ip: Present the VF VLAN tagging mode

Message ID 1572463033-26368-3-git-send-email-lariel@mellanox.com
State Superseded
Delegated to: David Ahern
Headers show
Series VGT+ support | expand

Commit Message

Ariel Levkovich Oct. 30, 2019, 7:17 p.m. UTC 
The change prints the VLAN tagging mode per VF on
link query.

The possible modes are:
1) VGT - Virtual Guest tagging mode.
2) VST - Virtual Switch tagging mode.
3) Trunk - Guest tagging mode with specific allowed VLAN access list.

For the full VLAN ids list in Trunk mode, it is required to
query the specific VF.

Signed-off-by: Ariel Levkovich <lariel@mellanox.com>
---
 include/uapi/linux/if_link.h | 14 ++++++++++++++
 ip/ipaddress.c               | 21 ++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

Comments

Stephen Hemminger Oct. 30, 2019, 11:29 p.m. UTC | #1 
On Wed, 30 Oct 2019 19:17:32 +0000
Ariel Levkovich <lariel@mellanox.com> wrote:

> +		if (vlan_mode->mode == IFLA_VF_VLAN_MODE_TRUNK)
> +			print_string(PRINT_ANY,
> +				     "vlan-mode",
> +				      ", vlan-mode %s",
> +				      "trunk");
> +		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VST)
> +			print_string(PRINT_ANY,
> +				     "vlan-mode",
> +				     ", vlan_mode %s",
> +				     "vst");
> +		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VGT)
> +			print_string(PRINT_ANY,
> +				     "vlan-mode",
> +				     ", vlan-mode %s",
> +				     "vgt");

This seems like you want something like:

		print_string(PRINT_ANY, "vlan-mode", ", vlan-mode %s",
				vlan_mode_str(vlan_mode->mode);

and why the comma in the output? the convention for ip commands is that
the output format is equivalent to the command line.
Ariel Levkovich Oct. 31, 2019, 2:50 p.m. UTC | #2 
On Wed, 30 Oct 2019 19:29:32 +0000
Stephen Hemminger <stephen@networkplumber.org> wrote
> On Wed, 30 Oct 2019 19:17:32 +0000
> Ariel Levkovich <lariel@mellanox.com> wrote:
> 
> > +		if (vlan_mode->mode == IFLA_VF_VLAN_MODE_TRUNK)
> > +			print_string(PRINT_ANY,
> > +				     "vlan-mode",
> > +				      ", vlan-mode %s",
> > +				      "trunk");
> > +		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VST)
> > +			print_string(PRINT_ANY,
> > +				     "vlan-mode",
> > +				     ", vlan_mode %s",
> > +				     "vst");
> > +		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VGT)
> > +			print_string(PRINT_ANY,
> > +				     "vlan-mode",
> > +				     ", vlan-mode %s",
> > +				     "vgt");
> 
> This seems like you want something like:
> 
> 		print_string(PRINT_ANY, "vlan-mode", ", vlan-mode %s",
> 				vlan_mode_str(vlan_mode->mode);
> 
> and why the comma in the output? the convention for ip commands is that
> the output format is equivalent to the command line.
Thanks for your review Stephen.
I'll fix the if/else to the suggested line.
I see the comma is used when presenting other VF attributes as well like link-state, spoof, trust.
So what is the right convention here?
diff mbox series

Patch

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d39017b..6304add 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -697,6 +697,7 @@  enum {
 	IFLA_VF_IB_PORT_GUID,	/* VF Infiniband port GUID */
 	IFLA_VF_VLAN_LIST,	/* nested list of vlans, option for QinQ */
 	IFLA_VF_BROADCAST,	/* VF broadcast */
+	IFLA_VF_VLAN_MODE,	/* vlan tagging mode */
 	__IFLA_VF_MAX,
 };
 
@@ -711,6 +712,19 @@  struct ifla_vf_broadcast {
 	__u8 broadcast[32];
 };
 
+enum {
+	IFLA_VF_VLAN_MODE_UNSPEC,
+	IFLA_VF_VLAN_MODE_VGT,
+	IFLA_VF_VLAN_MODE_VST,
+	IFLA_VF_VLAN_MODE_TRUNK,
+	__IFLA_VF_VLAN_MODE_MAX,
+};
+
+struct ifla_vf_vlan_mode {
+	__u32 vf;
+	__u32 mode; /* The VLAN tagging mode */
+};
+
 struct ifla_vf_vlan {
 	__u32 vf;
 	__u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 0521fdc..a66ca02 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -357,7 +357,6 @@  static void print_vfinfo(FILE *fp, struct ifinfomsg *ifi, struct rtattr *vfinfo)
 	struct ifla_vf_broadcast *vf_broadcast;
 	struct ifla_vf_tx_rate *vf_tx_rate;
 	struct rtattr *vf[IFLA_VF_MAX + 1] = {};
-
 	SPRINT_BUF(b1);
 
 	if (vfinfo->rta_type != IFLA_VF_INFO) {
@@ -404,6 +403,26 @@  static void print_vfinfo(FILE *fp, struct ifinfomsg *ifi, struct rtattr *vfinfo)
 					       b1, sizeof(b1)));
 	}
 
+	if (vf[IFLA_VF_VLAN_MODE]) {
+		struct ifla_vf_vlan_mode *vlan_mode = RTA_DATA(vf[IFLA_VF_VLAN_MODE]);
+
+		if (vlan_mode->mode == IFLA_VF_VLAN_MODE_TRUNK)
+			print_string(PRINT_ANY,
+				     "vlan-mode",
+				      ", vlan-mode %s",
+				      "trunk");
+		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VST)
+			print_string(PRINT_ANY,
+				     "vlan-mode",
+				     ", vlan_mode %s",
+				     "vst");
+		else if (vlan_mode->mode == IFLA_VF_VLAN_MODE_VGT)
+			print_string(PRINT_ANY,
+				     "vlan-mode",
+				     ", vlan-mode %s",
+				     "vgt");
+	}
+
 	if (vf[IFLA_VF_VLAN_LIST]) {
 		struct rtattr *i, *vfvlanlist = vf[IFLA_VF_VLAN_LIST];
 		int rem = RTA_PAYLOAD(vfvlanlist);