| Message ID | 20120214192627.GA14163@akhaparde-VBox |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2012-02-14 at 13:26 -0600, Ajit Khaparde wrote: > Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com> > --- > 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.
> -----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: > > Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com> > > --- > > 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. > > > + 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. > > > + 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? Yes. This is for Rx filtering. Tx filtering privileges can be added if needed. > > > + 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). > > > + 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. > > > + __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.
From: <Ajit.Khaparde@Emulex.Com> Date: Tue, 21 Feb 2012 14:02:27 -0800 >> -----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: >> > +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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2012-02-21 at 17:04 -0500, David Miller wrote: > From: <Ajit.Khaparde@Emulex.Com> > Date: Tue, 21 Feb 2012 14:02:27 -0800 > > >> -----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: > >> > +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.
> -----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: <Ajit.Khaparde@Emulex.Com> > Date: Tue, 21 Feb 2012 14:02:27 -0800 > > >> -----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: > >> > +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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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: > > From: <Ajit.Khaparde@Emulex.Com> > > Date: Tue, 21 Feb 2012 14:02:27 -0800 > > > > >> -----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: > > >> > +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.
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;
Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com> --- include/linux/if_link.h | 21 +++++++++++++++++++++ include/linux/netdevice.h | 3 +++ net/core/rtnetlink.c | 17 ++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletions(-)