[RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[Jeff Kirsher]

From: Williams, Mitch A <mitch.a.williams@intel.com>

This patch adds support to the "ip" tool for setting the MAC address and
VLAN filter for hardware device queues. This is most immediately useful for
SR-IOV; for VF devices to be usable in the real world, the hypervisor or VM
manager must be able to set these parameters before the VF device is
assigned to any VM.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 include/linux/if_link.h |   16 ++++++++++++++++
 ip/iplink.c             |   29 +++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index b0b9e8a..a458d63 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -81,6 +81,8 @@ enum
 #define IFLA_LINKINFO IFLA_LINKINFO
 	IFLA_NET_NS_PID,
 	IFLA_IFALIAS,
+	IFLA_QUEUE_MAC,		/* Hardware queue specific attributes */
+	IFLA_QUEUE_VLAN,
 	__IFLA_MAX
 };
 
@@ -188,4 +190,18 @@ struct ifla_vlan_qos_mapping
 	__u32 to;
 };
 
+/* subqueue managment section */
+
+struct ifla_queue_mac
+{
+	__u32 queue;
+	__u8 mac[32]; /* MAX_ADDR_LEN */
+};
+
+struct ifla_queue_vlan
+{
+	__u32 queue;
+	__u32 vlan;
+};
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/iplink.c b/ip/iplink.c
index 32cce24..9542d62 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -68,7 +68,9 @@ void iplink_usage(void)
 	fprintf(stderr, "	                  [ mtu MTU ]\n");
 	fprintf(stderr, "	                  [ netns PID ]\n");
 	fprintf(stderr, "			  [ alias NAME ]\n");
-	fprintf(stderr, "       ip link show [ DEVICE ]\n");
+	fprintf(stderr, "	                  [ queue NUM [ mac LLADDR ]\n");
+	fprintf(stderr, "				      [ vlan VLANID ] ] \n");
+	fprintf(stderr, "       ip link show [ DEVICE [ queue NUM] ]\n");
 
 	if (iplink_have_newlink()) {
 		fprintf(stderr, "\n");
@@ -176,7 +178,7 @@ struct iplink_req {
 int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		char **name, char **type, char **link, char **dev)
 {
-	int ret, len;
+	int ret, len, qnum;
 	char abuf[32];
 	int qlen = -1;
 	int mtu = -1;
@@ -278,6 +280,29 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				req->i.ifi_flags |= IFF_NOARP;
 			} else
 				return on_off("noarp");
+		} else if (strcmp(*argv, "queue") == 0) {
+			NEXT_ARG();
+			if (get_integer(&qnum,  *argv, 0)) {
+				invarg("Invalid \"queue\" value\n", *argv);
+			}
+			NEXT_ARG();
+			if (matches(*argv, "mac") == 0) {
+				struct ifla_queue_mac iqm;
+				NEXT_ARG();
+				iqm.queue = qnum;
+				len = ll_addr_a2n((char *)iqm.mac, 32, *argv);
+				if (len < 0)
+					return -1;
+				addattr_l(&req->n, sizeof(*req), IFLA_QUEUE_MAC, &iqm, sizeof(iqm));
+			} else if (matches(*argv, "vlan") == 0) {
+				struct ifla_queue_vlan iqv;
+				NEXT_ARG();
+				if (get_unsigned(&iqv.vlan, *argv, 0)) {
+					invarg("Invalid \"vlan\" value\n", *argv);
+				}
+				iqv.queue = qnum;
+				addattr_l(&req->n, sizeof(*req), IFLA_QUEUE_VLAN, &iqv, sizeof(iqv));
+			}
 #ifdef IFF_DYNAMIC
 		} else if (matches(*argv, "dynamic") == 0) {
 			NEXT_ARG();

Re: [RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[Stephen Hemminger]

On Tue, 17 Nov 2009 13:55:07 -0800
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> From: Williams, Mitch A <mitch.a.williams@intel.com>
> 
> This patch adds support to the "ip" tool for setting the MAC address and
> VLAN filter for hardware device queues. This is most immediately useful for
> SR-IOV; for VF devices to be usable in the real world, the hypervisor or VM
> manager must be able to set these parameters before the VF device is
> assigned to any VM.
> 
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Is there anything to avoid prevent this from being misused by users who
are doing multiqueue. Maybe we need equivalent of "mounted" flag that block
devices have?

Re: [RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 17 Nov 2009 14:06:41 -0800

> On Tue, 17 Nov 2009 13:55:07 -0800
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
>> From: Williams, Mitch A <mitch.a.williams@intel.com>
>> 
>> This patch adds support to the "ip" tool for setting the MAC address and
>> VLAN filter for hardware device queues. This is most immediately useful for
>> SR-IOV; for VF devices to be usable in the real world, the hypervisor or VM
>> manager must be able to set these parameters before the VF device is
>> assigned to any VM.
>> 
>> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> Is there anything to avoid prevent this from being misused by users who
> are doing multiqueue. Maybe we need equivalent of "mounted" flag that block
> devices have?

It's a privileged config operation as far as I can tell.

Given that, what could we possibly need to protect?

This stuff looks basically fine to me.

Re: [RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[Stephen Hemminger]

On Wed, 18 Nov 2009 10:07:28 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 17 Nov 2009 14:06:41 -0800
> 
> > On Tue, 17 Nov 2009 13:55:07 -0800
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> > 
> >> From: Williams, Mitch A <mitch.a.williams@intel.com>
> >> 
> >> This patch adds support to the "ip" tool for setting the MAC address and
> >> VLAN filter for hardware device queues. This is most immediately useful for
> >> SR-IOV; for VF devices to be usable in the real world, the hypervisor or VM
> >> manager must be able to set these parameters before the VF device is
> >> assigned to any VM.
> >> 
> >> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> >> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > 
> > Is there anything to avoid prevent this from being misused by users who
> > are doing multiqueue. Maybe we need equivalent of "mounted" flag that block
> > devices have?
> 
> It's a privileged config operation as far as I can tell.
> 
> Given that, what could we possibly need to protect?
> 
> This stuff looks basically fine to me.
> 

I was thinking that maybe the general question of SR-IOV overlap with other
multiqueue usage. How is it possible to be sure queue is not being used
for other traffic?  The MAC stuff itself is fine, just an example where
changing a queue being used for SR-IOV makes sense, but if being used
for regular multiqueue receive doesn't.

The filesystem example is that for years it was possible to do something
dumb like do fsck on a mounted filesystem and cause trouble (on unix and early
linux); but current systems don't allow it because it is stupid idea.

-- 

RE: [RFC PATCH iproute2] ip: Add support for setting MAC and VLAN on hardware queues

[Williams, Mitch A]

>From: Stephen Hemminger [mailto:shemminger@vyatta.com]
>Sent: Wednesday, November 18, 2009 11:16 AM
>> >
>> > Is there anything to avoid prevent this from being misused by users who
>> > are doing multiqueue. Maybe we need equivalent of "mounted" flag that
>block
>> > devices have?
>>
>> It's a privileged config operation as far as I can tell.
>>
>> Given that, what could we possibly need to protect?
>>
>> This stuff looks basically fine to me.
>>
>
>I was thinking that maybe the general question of SR-IOV overlap with other
>multiqueue usage. How is it possible to be sure queue is not being used
>for other traffic?  The MAC stuff itself is fine, just an example where
>changing a queue being used for SR-IOV makes sense, but if being used
>for regular multiqueue receive doesn't.
>
>The filesystem example is that for years it was possible to do something
>dumb like do fsck on a mounted filesystem and cause trouble (on unix and
>early
>linux); but current systems don't allow it because it is stupid idea.
>

Right now, this is only intended for SR-IOV usage, and the ixgbe driver enforces that by returning error if you try to set filters on queue 0, which corresponds to the PF device. We could conceivably extend this for use with non-IOV device queues and filters, but we'd need to look long and hard at the use cases and semantics of "what is a queue?" before we do that.

In this case (SR-IOV), we really can't realistically do any policy enforcement aside from root privilege.  Yeah, it's a bad idea to change these filters on an active queue - the VM would just mysteriously stop receiving traffic.  OTOH, the driver can't tell for sure what's going on with the VF device. Is it running, unloaded, crashed, booting...? There's no way to tell.

So we have to allow the filters to be changed at any time, even if there's a possibility that the VM is using the queue. We have to trust that the VM manager/hypervisor/whatever knows what it's doing, because we have no choice.

-Mitch