[RFC net-next 1/2] if_link : add support for VF privileges

Subsystems: networking drivers, networking [general], the rest

7 messages, 3 authors, 2012-03-01 · open the first message on its own page

[RFC net-next 1/2] if_link : add support for VF privileges

From: Ajit Khaparde <hidden>
Date: 2012-02-14 19:27:00

Signed-off-by: Ajit Khaparde <redacted>
---
 include/linux/if_link.h   |   21 +++++++++++++++++++++
 include/linux/netdevice.h |    3 +++
 net/core/rtnetlink.c      |   17 ++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index c52d4b5..9c93a8e 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -280,11 +280,26 @@ enum {
 	IFLA_VF_VLAN,
 	IFLA_VF_TX_RATE,	/* TX Bandwidth Allocation */
 	IFLA_VF_SPOOFCHK,	/* Spoof Checking on/off switch */
+	IFLA_VF_PRIVILEGE,	/* VF Privilege level setting */
 	__IFLA_VF_MAX,
 };
 
 #define IFLA_VF_MAX (__IFLA_VF_MAX - 1)
 
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
+	IFLA_VF_PRIVILEGE_STATS	= 2,	/* Privilege to gather statistics */
+	IFLA_VF_PRIVILEGE_LNK_MGMT = 4,	/* Privilege to manage link params */
+	IFLA_VF_PRIVILEGE_DIAG	= 8,	/* Privilege to perform diagnostics */
+	IFLA_VF_PRIVILEGE_MAC	= 16,	/* Privilege to modify MAC address */
+	IFLA_VF_PRIVILEGE_VLAN	= 32,	/* Privilege to add or remove VLANs */
+	IFLA_VF_PRIVILEGE_DEV_CFG = 64,	/* Unrestricted admin access privlege */
+	IFLA_VF_PRIVILEGE_SECURE = 128,	/* Privilege to access secure content */
+	__IFLA_VF_PRIVILEGE_MAX,
+};
+
+#define IFLA_VF_PRIVILEGE_MAX (__IFLA_VF_PRIVILEGE_MAX - 1)
+
 struct ifla_vf_mac {
 	__u32 vf;
 	__u8 mac[32]; /* MAX_ADDR_LEN */
@@ -305,6 +320,11 @@ struct ifla_vf_spoofchk {
 	__u32 vf;
 	__u32 setting;
 };
+
+struct ifla_vf_privilege {
+	__u32 vf;
+	__u32 privilege;
+};
 #ifdef __KERNEL__
 
 /* We don't want this structure exposed to user space */
@@ -315,6 +335,7 @@ struct ifla_vf_info {
 	__u32 qos;
 	__u32 tx_rate;
 	__u32 spoofchk;
+	__u32 privilege;
 };
 #endif
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0eac07c..f80cab7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -823,6 +823,7 @@ struct netdev_fcoe_hbainfo {
  * int (*ndo_set_vf_vlan)(struct net_device *dev, int vf, u16 vlan, u8 qos);
  * int (*ndo_set_vf_tx_rate)(struct net_device *dev, int vf, int rate);
  * int (*ndo_set_vf_spoofchk)(struct net_device *dev, int vf, bool setting);
+ * int (*ndo_set_vf_privilege)(struct net_device *dev, int vf, u32 privilege);
  * int (*ndo_get_vf_config)(struct net_device *dev,
  *			    int vf, struct ifla_vf_info *ivf);
  * int (*ndo_set_vf_port)(struct net_device *dev, int vf,
@@ -952,6 +953,8 @@ struct net_device_ops {
 						      int vf, int rate);
 	int			(*ndo_set_vf_spoofchk)(struct net_device *dev,
 						       int vf, bool setting);
+	int			(*ndo_set_vf_privilege)(struct net_device *dev,
+							int vf, u32 privilege);
 	int			(*ndo_get_vf_config)(struct net_device *dev,
 						     int vf,
 						     struct ifla_vf_info *ivf);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 65aebd4..061d8e3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -959,6 +959,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			struct ifla_vf_vlan vf_vlan;
 			struct ifla_vf_tx_rate vf_tx_rate;
 			struct ifla_vf_spoofchk vf_spoofchk;
+			struct ifla_vf_privilege vf_privilege;
 
 			/*
 			 * Not all SR-IOV capable drivers support the
@@ -967,18 +968,21 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			 * report anything.
 			 */
 			ivi.spoofchk = -1;
+			ivi.privilege = -1;
 			if (dev->netdev_ops->ndo_get_vf_config(dev, i, &ivi))
 				break;
 			vf_mac.vf =
 				vf_vlan.vf =
 				vf_tx_rate.vf =
-				vf_spoofchk.vf = ivi.vf;
+				vf_spoofchk.vf =
+				vf_privilege.vf = ivi.vf;
 
 			memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
 			vf_vlan.vlan = ivi.vlan;
 			vf_vlan.qos = ivi.qos;
 			vf_tx_rate.rate = ivi.tx_rate;
 			vf_spoofchk.setting = ivi.spoofchk;
+			vf_privilege.privilege = ivi.privilege;
 			vf = nla_nest_start(skb, IFLA_VF_INFO);
 			if (!vf) {
 				nla_nest_cancel(skb, vfinfo);
@@ -990,6 +994,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 				&vf_tx_rate);
 			NLA_PUT(skb, IFLA_VF_SPOOFCHK, sizeof(vf_spoofchk),
 				&vf_spoofchk);
+			NLA_PUT(skb, IFLA_VF_PRIVILEGE, sizeof(vf_privilege),
+				&vf_privilege);
 			nla_nest_end(skb, vf);
 		}
 		nla_nest_end(skb, vfinfo);
@@ -1232,6 +1238,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 							       ivs->setting);
 			break;
 		}
+		case IFLA_VF_PRIVILEGE: {
+			struct ifla_vf_privilege *ivp;
+			ivp = nla_data(vf);
+			err = -EOPNOTSUPP;
+			if (ops->ndo_set_vf_privilege)
+				err = ops->ndo_set_vf_privilege(dev, ivp->vf,
+							       ivp->privilege);
+			break;
+		}
 		default:
 			err = -EINVAL;
 			break;
-- 
1.7.1

Re: [RFC net-next 1/2] if_link : add support for VF privileges

From: Ben Hutchings <hidden>
Date: 2012-02-14 23:25:00

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted hunk
Signed-off-by: Ajit Khaparde <redacted>
---
 include/linux/if_link.h   |   21 +++++++++++++++++++++
 include/linux/netdevice.h |    3 +++
 net/core/rtnetlink.c      |   17 ++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index c52d4b5..9c93a8e 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -280,11 +280,26 @@ enum {
 	IFLA_VF_VLAN,
 	IFLA_VF_TX_RATE,	/* TX Bandwidth Allocation */
 	IFLA_VF_SPOOFCHK,	/* Spoof Checking on/off switch */
+	IFLA_VF_PRIVILEGE,	/* VF Privilege level setting */
 	__IFLA_VF_MAX,
 };
 
 #define IFLA_VF_MAX (__IFLA_VF_MAX - 1)
 
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
+	IFLA_VF_PRIVILEGE_STATS	= 2,	/* Privilege to gather statistics */
I assume that means port or board statistics as opposed to statistics
for the VF?
+	IFLA_VF_PRIVILEGE_LNK_MGMT = 4,	/* Privilege to manage link params */
+	IFLA_VF_PRIVILEGE_DIAG	= 8,	/* Privilege to perform diagnostics */
+	IFLA_VF_PRIVILEGE_MAC	= 16,	/* Privilege to modify MAC address */
+	IFLA_VF_PRIVILEGE_VLAN	= 32,	/* Privilege to add or remove VLANs */
I assume these two apply to RX filtering of the VF itself.  How about
control over TX filtering?
+	IFLA_VF_PRIVILEGE_DEV_CFG = 64,	/* Unrestricted admin access privlege */
Does that include all the other privileges, or does it mean 'everything
else'?
+	IFLA_VF_PRIVILEGE_SECURE = 128,	/* Privilege to access secure content */
What does that mean?
+	__IFLA_VF_PRIVILEGE_MAX,
+};
+
+#define IFLA_VF_PRIVILEGE_MAX (__IFLA_VF_PRIVILEGE_MAX - 1)
[...]

This doesn't seem to make sense for an enumeration of flags.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [RFC net-next 1/2] if_link : add support for VF privileges

From: <hidden>
Date: 2012-02-21 22:02:49

-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 14, 2012 5:25 PM
To: Khaparde, Ajit
Cc: davem@davemloft.net; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted
Signed-off-by: Ajit Khaparde <redacted>
---
 include/linux/if_link.h   |   21 +++++++++++++++++++++
 include/linux/netdevice.h |    3 +++
 net/core/rtnetlink.c      |   17 ++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index c52d4b5..9c93a8e 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -280,11 +280,26 @@ enum {
 	IFLA_VF_VLAN,
 	IFLA_VF_TX_RATE,	/* TX Bandwidth Allocation */
 	IFLA_VF_SPOOFCHK,	/* Spoof Checking on/off switch */
+	IFLA_VF_PRIVILEGE,	/* VF Privilege level setting */
 	__IFLA_VF_MAX,
 };

 #define IFLA_VF_MAX (__IFLA_VF_MAX - 1)

+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
Yes. Vendors can decide what privileges they want to grant for VFs by default.
quoted
+	IFLA_VF_PRIVILEGE_STATS	= 2,	/* Privilege to gather statistics */
I assume that means port or board statistics as opposed to statistics
for the VF?
Yes, Port statistics.
quoted
+	IFLA_VF_PRIVILEGE_LNK_MGMT = 4,	/* Privilege to manage link
params */
quoted
+	IFLA_VF_PRIVILEGE_DIAG	= 8,	/* Privilege to perform
diagnostics */
quoted
+	IFLA_VF_PRIVILEGE_MAC	= 16,	/* Privilege to modify MAC
address */
quoted
+	IFLA_VF_PRIVILEGE_VLAN	= 32,	/* Privilege to add or remove
VLANs */

I assume these two apply to RX filtering of the VF itself.  How about
control over TX filtering?
Yes. This is for Rx filtering. Tx filtering privileges can be added if needed.
quoted
+	IFLA_VF_PRIVILEGE_DEV_CFG = 64,	/* Unrestricted admin access
privlege */

Does that include all the other privileges, or does it mean 'everything
else'?
All other privileges with the exception of "secure content" (defined below).
quoted
+	IFLA_VF_PRIVILEGE_SECURE = 128,	/* Privilege to access secure
content */

What does that mean?
Privilege to access secure information on the ASIC like debug registers,
dump information.
quoted
+	__IFLA_VF_PRIVILEGE_MAX,
+};
+
+#define IFLA_VF_PRIVILEGE_MAX (__IFLA_VF_PRIVILEGE_MAX - 1)
[...]

This doesn't seem to make sense for an enumeration of flags.
Yes. Will take care.

Thanks
Ben.

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC net-next 1/2] if_link : add support for VF privileges

From: David Miller <davem@davemloft.net>
Date: 2012-02-21 22:04:47

From: <redacted>
Date: Tue, 21 Feb 2012 14:02:27 -0800
quoted
-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 14, 2012 5:25 PM
To: Khaparde, Ajit
Cc: davem@davemloft.net; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
Yes. Vendors can decide what privileges they want to grant for VFs by default.
That's terrible and a very bad interface for users.  It means every system
can have different defaults, from which we'll derive zero consistency.

Re: [RFC net-next 1/2] if_link : add support for VF privileges

From: Ben Hutchings <hidden>
Date: 2012-02-21 22:43:26

On Tue, 2012-02-21 at 17:04 -0500, David Miller wrote:
From: <redacted>
Date: Tue, 21 Feb 2012 14:02:27 -0800
quoted
quoted
-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 14, 2012 5:25 PM
To: Khaparde, Ajit
Cc: davem@davemloft.net; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
Yes. Vendors can decide what privileges they want to grant for VFs by default.
That's terrible and a very bad interface for users.  It means every system
can have different defaults, from which we'll derive zero consistency.
Aside from this, my concern is that if we assign privilege flags to
capabilities that VFs normally have now then the value reported where
the driver doesn't support this new operation should not be 0.

(Also, if a privilege is assigned by default, is it really a privilege?
Wouldn't 'capability' or 'permission' be a better term?)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [RFC net-next 1/2] if_link : add support for VF privileges

From: <hidden>
Date: 2012-03-01 18:53:44

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, February 21, 2012 4:05 PM
To: Khaparde, Ajit
Cc: bhutchings@solarflare.com; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

From: <redacted>
Date: Tue, 21 Feb 2012 14:02:27 -0800
quoted
quoted
-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 14, 2012 5:25 PM
To: Khaparde, Ajit
Cc: davem@davemloft.net; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
Yes. Vendors can decide what privileges they want to grant for VFs by default.
That's terrible and a very bad interface for users.  It means every system
can have different defaults, from which we'll derive zero consistency.
Since different hardware devices can have different capabilities this possibility exists.
Do you suggest indicating the individual capabilities in the bit mask rather than just a single bit?
That way every VF driver will get to indicate its capabilities.

RE: [RFC net-next 1/2] if_link : add support for VF privileges

From: <hidden>
Date: 2012-03-01 18:54:52

-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 21, 2012 4:43 PM
To: David Miller
Cc: Khaparde, Ajit; shemminger@linux-foundation.org; netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-21 at 17:04 -0500, David Miller wrote:
quoted
From: <redacted>
Date: Tue, 21 Feb 2012 14:02:27 -0800
quoted
quoted
-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, February 14, 2012 5:25 PM
To: Khaparde, Ajit
Cc: davem@davemloft.net; shemminger@linux-foundation.org;
netdev@vger.kernel.org
Subject: Re: [RFC net-next 1/2] if_link : add support for VF privileges

On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote:
quoted
+enum {
+	IFLA_VF_PRIVILEGE_DEFAULT = 1,	/* Default privileges */
What are the default privileges?  Should existing drivers report that
their VFs have this?
Yes. Vendors can decide what privileges they want to grant for VFs by default.
That's terrible and a very bad interface for users.  It means every system
can have different defaults, from which we'll derive zero consistency.
Aside from this, my concern is that if we assign privilege flags to
capabilities that VFs normally have now then the value reported where
the driver doesn't support this new operation should not be 0.
 
Agree.
(Also, if a privilege is assigned by default, is it really a privilege?
Wouldn't 'capability' or 'permission' be a better term?)
Sure. Anything is fine.
Ben.

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help