[PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

This adds support for setting synthetic NIC MAC address from within Linux
guests. Before using this feature, the option "spoofing of MAC address"
should be enabled at the Hyper-V manager / Settings of the synthetic
NIC.

Thanks to Kin Cho <kcho@infoblox.com> for the initial implementation and
tests. And, thanks to Long Li <longli@microsoft.com> for the debugging
works.

Reported-and-tested-by: Kin Cho <kcho@infoblox.com>
Reported-by: Long Li <longli@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc_drv.c   |   30 +++++++++++++-
 drivers/net/hyperv/rndis_filter.c |   79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 2857ab0..95ceb35 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -131,6 +131,7 @@ int rndis_filter_send(struct hv_device *dev,
 			struct hv_netvsc_packet *pkt);
 
 int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
 
 
 #define NVSP_INVALID_PROTOCOL_VERSION	((u32)0xFFFFFFFF)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 8f8ed33..8e23c08 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -341,6 +341,34 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	return 0;
 }
 
+
+static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
+{
+	struct net_device_context *ndevctx = netdev_priv(ndev);
+	struct hv_device *hdev =  ndevctx->device_ctx;
+	struct sockaddr *addr = p;
+	char save_adr[14];
+	unsigned char save_aatype;
+	int err;
+
+	memcpy(save_adr, ndev->dev_addr, ETH_ALEN);
+	save_aatype = ndev->addr_assign_type;
+
+	err = eth_mac_addr(ndev, p);
+	if (err != 0)
+		return err;
+
+	err = rndis_filter_set_device_mac(hdev, addr->sa_data);
+	if (err != 0) {
+		/* roll back to saved MAC */
+		memcpy(ndev->dev_addr, save_adr, ETH_ALEN);
+		ndev->addr_assign_type = save_aatype;
+	}
+
+	return err;
+}
+
+
 static const struct ethtool_ops ethtool_ops = {
 	.get_drvinfo	= netvsc_get_drvinfo,
 	.get_link	= ethtool_op_get_link,
@@ -353,7 +381,7 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_rx_mode =		netvsc_set_multicast_list,
 	.ndo_change_mtu =		netvsc_change_mtu,
 	.ndo_validate_addr =		eth_validate_addr,
-	.ndo_set_mac_address =		eth_mac_addr,
+	.ndo_set_mac_address =		netvsc_set_mac_addr,
 };
 
 /*
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 981ebb1..fbf5394 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -27,6 +27,7 @@
 #include <linux/if_ether.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
+#include <linux/nls.h>
 
 #include "hyperv_net.h"
 
@@ -47,6 +48,7 @@ struct rndis_request {
 	struct hv_page_buffer buf;
 	/* FIXME: We assumed a fixed size request here. */
 	struct rndis_message request_msg;
+	u8 ext[100];
 };
 
 static void rndis_filter_send_completion(void *ctx);
@@ -511,6 +513,83 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev)
 				      dev->hw_mac_adr, &size);
 }
 
+#define NWADR_STR "NetworkAddress"
+#define NWADR_STRLEN 14
+
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
+{
+	struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+	struct rndis_device *rdev = nvdev->extension;
+	struct net_device *ndev = nvdev->ndev;
+	struct rndis_request *request;
+	struct rndis_set_request *set;
+	struct rndis_config_parameter_info *cpi;
+	wchar_t *cfg_nwadr, *cfg_mac;
+	struct rndis_set_complete *set_complete;
+	char macstr[2*ETH_ALEN+1];
+	u32 extlen = sizeof(struct rndis_config_parameter_info) +
+		2*NWADR_STRLEN + 4*ETH_ALEN;
+	int ret, t;
+
+	request = get_rndis_request(rdev, RNDIS_MSG_SET,
+		RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen);
+	if (!request)
+		return -ENOMEM;
+
+	set = &request->request_msg.msg.set_req;
+	set->oid = RNDIS_OID_GEN_RNDIS_CONFIG_PARAMETER;
+	set->info_buflen = extlen;
+	set->info_buf_offset = sizeof(struct rndis_set_request);
+	set->dev_vc_handle = 0;
+
+	cpi = (struct rndis_config_parameter_info *)((ulong)set +
+		set->info_buf_offset);
+	cpi->parameter_name_offset =
+		sizeof(struct rndis_config_parameter_info);
+	/* Multiply by 2 because host needs 2 bytes (utf16) for each char */
+	cpi->parameter_name_length = 2*NWADR_STRLEN;
+	cpi->parameter_type = RNDIS_CONFIG_PARAM_TYPE_STRING;
+	cpi->parameter_value_offset =
+		cpi->parameter_name_offset + cpi->parameter_name_length;
+	/* Multiply by 4 because each MAC byte displayed as 2 utf16 chars */
+	cpi->parameter_value_length = 4*ETH_ALEN;
+
+	cfg_nwadr = (wchar_t *)((ulong)cpi + cpi->parameter_name_offset);
+	cfg_mac = (wchar_t *)((ulong)cpi + cpi->parameter_value_offset);
+	ret = utf8s_to_utf16s(NWADR_STR, NWADR_STRLEN, UTF16_HOST_ENDIAN,
+			      cfg_nwadr, NWADR_STRLEN);
+	if (ret < 0)
+		goto cleanup;
+	snprintf(macstr, 2*ETH_ALEN+1, "%pm", mac);
+	ret = utf8s_to_utf16s(macstr, 2*ETH_ALEN, UTF16_HOST_ENDIAN,
+			      cfg_mac, 2*ETH_ALEN);
+	if (ret < 0)
+		goto cleanup;
+
+	ret = rndis_filter_send_request(rdev, request);
+	if (ret != 0)
+		goto cleanup;
+
+	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+	if (t == 0) {
+		netdev_err(ndev, "timeout before we got a set response...\n");
+		/*
+		 * can't put_rndis_request, since we may still receive a
+		 * send-completion.
+		 */
+		return -EBUSY;
+	} else {
+		set_complete = &request->response_msg.msg.set_complete;
+		if (set_complete->status != RNDIS_STATUS_SUCCESS)
+			ret = -EINVAL;
+	}
+
+cleanup:
+	put_rndis_request(rdev, request);
+	return ret;
+}
+
+
 static int rndis_filter_query_device_link_status(struct rndis_device *dev)
 {
 	u32 size = sizeof(u32);
-- 
1.7.4.1

[PATCH net, 1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

This adds support for setting synthetic NIC MAC address from within Linux
guests. Before using this feature, the option "spoofing of MAC address"
should be enabled at the Hyper-V manager / Settings of the synthetic
NIC.

Thanks to Kin Cho <kcho@infoblox.com> for the initial implementation and
tests. And, thanks to Long Li <longli@microsoft.com> for the debugging
works.

Reported-and-tested-by: Kin Cho <kcho@infoblox.com>
Reported-by: Long Li <longli@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc_drv.c   |   30 +++++++++++++-
 drivers/net/hyperv/rndis_filter.c |   79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 2857ab0..95ceb35 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -131,6 +131,7 @@ int rndis_filter_send(struct hv_device *dev,
 			struct hv_netvsc_packet *pkt);
 
 int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
 
 
 #define NVSP_INVALID_PROTOCOL_VERSION	((u32)0xFFFFFFFF)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 8f8ed33..8e23c08 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -341,6 +341,34 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	return 0;
 }
 
+
+static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
+{
+	struct net_device_context *ndevctx = netdev_priv(ndev);
+	struct hv_device *hdev =  ndevctx->device_ctx;
+	struct sockaddr *addr = p;
+	char save_adr[14];
+	unsigned char save_aatype;
+	int err;
+
+	memcpy(save_adr, ndev->dev_addr, ETH_ALEN);
+	save_aatype = ndev->addr_assign_type;
+
+	err = eth_mac_addr(ndev, p);
+	if (err != 0)
+		return err;
+
+	err = rndis_filter_set_device_mac(hdev, addr->sa_data);
+	if (err != 0) {
+		/* roll back to saved MAC */
+		memcpy(ndev->dev_addr, save_adr, ETH_ALEN);
+		ndev->addr_assign_type = save_aatype;
+	}
+
+	return err;
+}
+
+
 static const struct ethtool_ops ethtool_ops = {
 	.get_drvinfo	= netvsc_get_drvinfo,
 	.get_link	= ethtool_op_get_link,
@@ -353,7 +381,7 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_rx_mode =		netvsc_set_multicast_list,
 	.ndo_change_mtu =		netvsc_change_mtu,
 	.ndo_validate_addr =		eth_validate_addr,
-	.ndo_set_mac_address =		eth_mac_addr,
+	.ndo_set_mac_address =		netvsc_set_mac_addr,
 };
 
 /*
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 981ebb1..fbf5394 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -27,6 +27,7 @@
 #include <linux/if_ether.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
+#include <linux/nls.h>
 
 #include "hyperv_net.h"
 
@@ -47,6 +48,7 @@ struct rndis_request {
 	struct hv_page_buffer buf;
 	/* FIXME: We assumed a fixed size request here. */
 	struct rndis_message request_msg;
+	u8 ext[100];
 };
 
 static void rndis_filter_send_completion(void *ctx);
@@ -511,6 +513,83 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev)
 				      dev->hw_mac_adr, &size);
 }
 
+#define NWADR_STR "NetworkAddress"
+#define NWADR_STRLEN 14
+
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
+{
+	struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+	struct rndis_device *rdev = nvdev->extension;
+	struct net_device *ndev = nvdev->ndev;
+	struct rndis_request *request;
+	struct rndis_set_request *set;
+	struct rndis_config_parameter_info *cpi;
+	wchar_t *cfg_nwadr, *cfg_mac;
+	struct rndis_set_complete *set_complete;
+	char macstr[2*ETH_ALEN+1];
+	u32 extlen = sizeof(struct rndis_config_parameter_info) +
+		2*NWADR_STRLEN + 4*ETH_ALEN;
+	int ret, t;
+
+	request = get_rndis_request(rdev, RNDIS_MSG_SET,
+		RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen);
+	if (!request)
+		return -ENOMEM;
+
+	set = &request->request_msg.msg.set_req;
+	set->oid = RNDIS_OID_GEN_RNDIS_CONFIG_PARAMETER;
+	set->info_buflen = extlen;
+	set->info_buf_offset = sizeof(struct rndis_set_request);
+	set->dev_vc_handle = 0;
+
+	cpi = (struct rndis_config_parameter_info *)((ulong)set +
+		set->info_buf_offset);
+	cpi->parameter_name_offset =
+		sizeof(struct rndis_config_parameter_info);
+	/* Multiply by 2 because host needs 2 bytes (utf16) for each char */
+	cpi->parameter_name_length = 2*NWADR_STRLEN;
+	cpi->parameter_type = RNDIS_CONFIG_PARAM_TYPE_STRING;
+	cpi->parameter_value_offset =
+		cpi->parameter_name_offset + cpi->parameter_name_length;
+	/* Multiply by 4 because each MAC byte displayed as 2 utf16 chars */
+	cpi->parameter_value_length = 4*ETH_ALEN;
+
+	cfg_nwadr = (wchar_t *)((ulong)cpi + cpi->parameter_name_offset);
+	cfg_mac = (wchar_t *)((ulong)cpi + cpi->parameter_value_offset);
+	ret = utf8s_to_utf16s(NWADR_STR, NWADR_STRLEN, UTF16_HOST_ENDIAN,
+			      cfg_nwadr, NWADR_STRLEN);
+	if (ret < 0)
+		goto cleanup;
+	snprintf(macstr, 2*ETH_ALEN+1, "%pm", mac);
+	ret = utf8s_to_utf16s(macstr, 2*ETH_ALEN, UTF16_HOST_ENDIAN,
+			      cfg_mac, 2*ETH_ALEN);
+	if (ret < 0)
+		goto cleanup;
+
+	ret = rndis_filter_send_request(rdev, request);
+	if (ret != 0)
+		goto cleanup;
+
+	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+	if (t == 0) {
+		netdev_err(ndev, "timeout before we got a set response...\n");
+		/*
+		 * can't put_rndis_request, since we may still receive a
+		 * send-completion.
+		 */
+		return -EBUSY;
+	} else {
+		set_complete = &request->response_msg.msg.set_complete;
+		if (set_complete->status != RNDIS_STATUS_SUCCESS)
+			ret = -EINVAL;
+	}
+
+cleanup:
+	put_rndis_request(rdev, request);
+	return ret;
+}
+
+
 static int rndis_filter_query_device_link_status(struct rndis_device *dev)
 {
 	u32 size = sizeof(u32);
-- 
1.7.4.1

Re: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Tue, 10 Jul 2012 10:19:22 -0700

> This adds support for setting synthetic NIC MAC address from within Linux
> guests. Before using this feature, the option "spoofing of MAC address"
> should be enabled at the Hyper-V manager / Settings of the synthetic
> NIC.
> 
> Thanks to Kin Cho <kcho@infoblox.com> for the initial implementation and
> tests. And, thanks to Long Li <longli@microsoft.com> for the debugging
> works.
> 
> Reported-and-tested-by: Kin Cho <kcho@infoblox.com>
> Reported-by: Long Li <longli@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

Applied to net-next, thanks.

Re: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[David Miller]

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Tue, 10 Jul 2012 10:19:22 -0700

> This adds support for setting synthetic NIC MAC address from within Linux
> guests. Before using this feature, the option "spoofing of MAC address"
> should be enabled at the Hyper-V manager / Settings of the synthetic
> NIC.
> 
> Thanks to Kin Cho <kcho@infoblox.com> for the initial implementation and
> tests. And, thanks to Long Li <longli@microsoft.com> for the debugging
> works.
> 
> Reported-and-tested-by: Kin Cho <kcho@infoblox.com>
> Reported-by: Long Li <longli@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

Applied to net-next, thanks.

Re: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Olaf Hering]

On Tue, Jul 10, Haiyang Zhang wrote:

> diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
> index 981ebb1..fbf5394 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -47,6 +48,7 @@ struct rndis_request {
>  	struct hv_page_buffer buf;
>  	/* FIXME: We assumed a fixed size request here. */
>  	struct rndis_message request_msg;
> +	u8 ext[100];

This array is not referenced in the patch.
Please add a comment to the code what the purpose of this array is, and
why its size is 100 bytes.

Thanks.

Olaf

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1382 bytes --]



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 8:39 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Tue, Jul 10, Haiyang Zhang wrote:
> 
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index 981ebb1..fbf5394 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -47,6 +48,7 @@ struct rndis_request {
> >  	struct hv_page_buffer buf;
> >  	/* FIXME: We assumed a fixed size request here. */
> >  	struct rndis_message request_msg;
> > +	u8 ext[100];
> 
> This array is not referenced in the patch.
> Please add a comment to the code what the purpose of this array is, and why
> its size is 100 bytes.

It's a buffer for the extended info after the RNDIS message. It's referenced based
on the data offset in the RNDIS message. 100 byte size is enough for current needs, 
and should be sufficient for the near future.

I will add a comment to the code.

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 8:39 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Tue, Jul 10, Haiyang Zhang wrote:
> 
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index 981ebb1..fbf5394 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -47,6 +48,7 @@ struct rndis_request {
> >  	struct hv_page_buffer buf;
> >  	/* FIXME: We assumed a fixed size request here. */
> >  	struct rndis_message request_msg;
> > +	u8 ext[100];
> 
> This array is not referenced in the patch.
> Please add a comment to the code what the purpose of this array is, and why
> its size is 100 bytes.

It's a buffer for the extended info after the RNDIS message. It's referenced based
on the data offset in the RNDIS message. 100 byte size is enough for current needs, 
and should be sufficient for the near future.

I will add a comment to the code.

Thanks,
- Haiyang

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Ben Hutchings]

On Tue, 2012-07-10 at 17:03 +0000, Haiyang Zhang wrote:
> 
> > -----Original Message-----
> > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > Sent: Friday, July 06, 2012 8:19 PM
> > To: Haiyang Zhang
> > Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan;
> > olaf@aepfle.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org
> > Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> > within guests
> > 
> > On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> > > This adds support for setting synthetic NIC MAC address from within
> > Linux
> > > guests. Before using this feature, the option "spoofing of MAC
> > address"
> > > should be enabled at the Hyper-V manager / Settings of the synthetic
> > > NIC.
> > [...]
> > > +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
> > > +{
> > [...]
> > > +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> > > +	if (t == 0) {
> > > +		netdev_err(ndev, "timeout before we got a set
> > response...\n");
> > > +		/*
> > > +		 * can't put_rndis_request, since we may still receive a
> > > +		 * send-completion.
> > > +		 */
> > > +		return -EBUSY;
> > > +	} else {
> > > +		set_complete = &request->response_msg.msg.set_complete;
> > > +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> > > +			ret = -EINVAL;
> > [...]
> > 
> > Is there a specific error code that indicates the hypervisor is
> > configured not to allow MAC address changes?  If so, shouldn't that be
> > translated to return EPERM rather than EINVAL?
> 
> I have check the return code, 0xc000000d, which is returned both when MAC
> spoofing is not enabled or the parameter contains other errors. So we can't
> tell if it permission error or not. I will re-submit this patch still
> using EINVAL.

Oh well, thanks for trying!

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: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1840 bytes --]



> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, July 06, 2012 8:19 PM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan;
> olaf@aepfle.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> > This adds support for setting synthetic NIC MAC address from within
> Linux
> > guests. Before using this feature, the option "spoofing of MAC
> address"
> > should be enabled at the Hyper-V manager / Settings of the synthetic
> > NIC.
> [...]
> > +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
> > +{
> [...]
> > +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> > +	if (t == 0) {
> > +		netdev_err(ndev, "timeout before we got a set
> response...\n");
> > +		/*
> > +		 * can't put_rndis_request, since we may still receive a
> > +		 * send-completion.
> > +		 */
> > +		return -EBUSY;
> > +	} else {
> > +		set_complete = &request->response_msg.msg.set_complete;
> > +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> > +			ret = -EINVAL;
> [...]
> 
> Is there a specific error code that indicates the hypervisor is
> configured not to allow MAC address changes?  If so, shouldn't that be
> translated to return EPERM rather than EINVAL?

I have check the return code, 0xc000000d, which is returned both when MAC
spoofing is not enabled or the parameter contains other errors. So we can't
tell if it permission error or not. I will re-submit this patch still
using EINVAL.

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, July 06, 2012 8:19 PM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan;
> olaf@aepfle.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> > This adds support for setting synthetic NIC MAC address from within
> Linux
> > guests. Before using this feature, the option "spoofing of MAC
> address"
> > should be enabled at the Hyper-V manager / Settings of the synthetic
> > NIC.
> [...]
> > +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
> > +{
> [...]
> > +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> > +	if (t == 0) {
> > +		netdev_err(ndev, "timeout before we got a set
> response...\n");
> > +		/*
> > +		 * can't put_rndis_request, since we may still receive a
> > +		 * send-completion.
> > +		 */
> > +		return -EBUSY;
> > +	} else {
> > +		set_complete = &request->response_msg.msg.set_complete;
> > +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> > +			ret = -EINVAL;
> [...]
> 
> Is there a specific error code that indicates the hypervisor is
> configured not to allow MAC address changes?  If so, shouldn't that be
> translated to return EPERM rather than EINVAL?

I have check the return code, 0xc000000d, which is returned both when MAC
spoofing is not enabled or the parameter contains other errors. So we can't
tell if it permission error or not. I will re-submit this patch still
using EINVAL.

Thanks,
- Haiyang

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1607 bytes --]



> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, July 06, 2012 8:19 PM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan;
> olaf@aepfle.de; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> > This adds support for setting synthetic NIC MAC address from within
> > Linux guests. Before using this feature, the option "spoofing of MAC
> address"
> > should be enabled at the Hyper-V manager / Settings of the synthetic
> > NIC.
> [...]
> > +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac) {
> [...]
> > +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> > +	if (t == 0) {
> > +		netdev_err(ndev, "timeout before we got a set
> response...\n");
> > +		/*
> > +		 * can't put_rndis_request, since we may still receive a
> > +		 * send-completion.
> > +		 */
> > +		return -EBUSY;
> > +	} else {
> > +		set_complete = &request->response_msg.msg.set_complete;
> > +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> > +			ret = -EINVAL;
> [...]
> 
> Is there a specific error code that indicates the hypervisor is configured not
> to allow MAC address changes?  If so, shouldn't that be translated to return
> EPERM rather than EINVAL?

Will do. 

Thanks,
- Haiyang

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, July 06, 2012 8:19 PM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan;
> olaf@aepfle.de; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> > This adds support for setting synthetic NIC MAC address from within
> > Linux guests. Before using this feature, the option "spoofing of MAC
> address"
> > should be enabled at the Hyper-V manager / Settings of the synthetic
> > NIC.
> [...]
> > +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac) {
> [...]
> > +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> > +	if (t == 0) {
> > +		netdev_err(ndev, "timeout before we got a set
> response...\n");
> > +		/*
> > +		 * can't put_rndis_request, since we may still receive a
> > +		 * send-completion.
> > +		 */
> > +		return -EBUSY;
> > +	} else {
> > +		set_complete = &request->response_msg.msg.set_complete;
> > +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> > +			ret = -EINVAL;
> [...]
> 
> Is there a specific error code that indicates the hypervisor is configured not
> to allow MAC address changes?  If so, shouldn't that be translated to return
> EPERM rather than EINVAL?

Will do. 

Thanks,
- Haiyang

Re: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Ben Hutchings]

On Fri, 2012-07-06 at 14:25 -0700, Haiyang Zhang wrote:
> This adds support for setting synthetic NIC MAC address from within Linux
> guests. Before using this feature, the option "spoofing of MAC address"
> should be enabled at the Hyper-V manager / Settings of the synthetic
> NIC.
[...]
> +int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
> +{
[...]
> +	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> +	if (t == 0) {
> +		netdev_err(ndev, "timeout before we got a set response...\n");
> +		/*
> +		 * can't put_rndis_request, since we may still receive a
> +		 * send-completion.
> +		 */
> +		return -EBUSY;
> +	} else {
> +		set_complete = &request->response_msg.msg.set_complete;
> +		if (set_complete->status != RNDIS_STATUS_SUCCESS)
> +			ret = -EINVAL;
[...]

Is there a specific error code that indicates the hypervisor is
configured not to allow MAC address changes?  If so, shouldn't that be
translated to return EPERM rather than EINVAL?

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.

[PATCH net,1/1] hyperv: Add support for setting MAC from within guests

[Haiyang Zhang]

This adds support for setting synthetic NIC MAC address from within Linux
guests. Before using this feature, the option "spoofing of MAC address"
should be enabled at the Hyper-V manager / Settings of the synthetic
NIC.

Thanks to Kin Cho <kcho@infoblox.com> for the initial implementation and
tests. And, thanks to Long Li <longli@microsoft.com> for the debugging
works.

Reported-and-tested-by: Kin Cho <kcho@infoblox.com>
Reported-by: Long Li <longli@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc_drv.c   |   30 +++++++++++++-
 drivers/net/hyperv/rndis_filter.c |   79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 2857ab0..95ceb35 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -131,6 +131,7 @@ int rndis_filter_send(struct hv_device *dev,
 			struct hv_netvsc_packet *pkt);
 
 int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
 
 
 #define NVSP_INVALID_PROTOCOL_VERSION	((u32)0xFFFFFFFF)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 8f8ed33..8e23c08 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -341,6 +341,34 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	return 0;
 }
 
+
+static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
+{
+	struct net_device_context *ndevctx = netdev_priv(ndev);
+	struct hv_device *hdev =  ndevctx->device_ctx;
+	struct sockaddr *addr = p;
+	char save_adr[14];
+	unsigned char save_aatype;
+	int err;
+
+	memcpy(save_adr, ndev->dev_addr, ETH_ALEN);
+	save_aatype = ndev->addr_assign_type;
+
+	err = eth_mac_addr(ndev, p);
+	if (err != 0)
+		return err;
+
+	err = rndis_filter_set_device_mac(hdev, addr->sa_data);
+	if (err != 0) {
+		/* roll back to saved MAC */
+		memcpy(ndev->dev_addr, save_adr, ETH_ALEN);
+		ndev->addr_assign_type = save_aatype;
+	}
+
+	return err;
+}
+
+
 static const struct ethtool_ops ethtool_ops = {
 	.get_drvinfo	= netvsc_get_drvinfo,
 	.get_link	= ethtool_op_get_link,
@@ -353,7 +381,7 @@ static const struct net_device_ops device_ops = {
 	.ndo_set_rx_mode =		netvsc_set_multicast_list,
 	.ndo_change_mtu =		netvsc_change_mtu,
 	.ndo_validate_addr =		eth_validate_addr,
-	.ndo_set_mac_address =		eth_mac_addr,
+	.ndo_set_mac_address =		netvsc_set_mac_addr,
 };
 
 /*
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 981ebb1..fbf5394 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -27,6 +27,7 @@
 #include <linux/if_ether.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
+#include <linux/nls.h>
 
 #include "hyperv_net.h"
 
@@ -47,6 +48,7 @@ struct rndis_request {
 	struct hv_page_buffer buf;
 	/* FIXME: We assumed a fixed size request here. */
 	struct rndis_message request_msg;
+	u8 ext[100];
 };
 
 static void rndis_filter_send_completion(void *ctx);
@@ -511,6 +513,83 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev)
 				      dev->hw_mac_adr, &size);
 }
 
+#define NWADR_STR "NetworkAddress"
+#define NWADR_STRLEN 14
+
+int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
+{
+	struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+	struct rndis_device *rdev = nvdev->extension;
+	struct net_device *ndev = nvdev->ndev;
+	struct rndis_request *request;
+	struct rndis_set_request *set;
+	struct rndis_config_parameter_info *cpi;
+	wchar_t *cfg_nwadr, *cfg_mac;
+	struct rndis_set_complete *set_complete;
+	char macstr[2*ETH_ALEN+1];
+	u32 extlen = sizeof(struct rndis_config_parameter_info) +
+		2*NWADR_STRLEN + 4*ETH_ALEN;
+	int ret, t;
+
+	request = get_rndis_request(rdev, RNDIS_MSG_SET,
+		RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen);
+	if (!request)
+		return -ENOMEM;
+
+	set = &request->request_msg.msg.set_req;
+	set->oid = RNDIS_OID_GEN_RNDIS_CONFIG_PARAMETER;
+	set->info_buflen = extlen;
+	set->info_buf_offset = sizeof(struct rndis_set_request);
+	set->dev_vc_handle = 0;
+
+	cpi = (struct rndis_config_parameter_info *)((ulong)set +
+		set->info_buf_offset);
+	cpi->parameter_name_offset =
+		sizeof(struct rndis_config_parameter_info);
+	/* Multiply by 2 because host needs 2 bytes (utf16) for each char */
+	cpi->parameter_name_length = 2*NWADR_STRLEN;
+	cpi->parameter_type = RNDIS_CONFIG_PARAM_TYPE_STRING;
+	cpi->parameter_value_offset =
+		cpi->parameter_name_offset + cpi->parameter_name_length;
+	/* Multiply by 4 because each MAC byte displayed as 2 utf16 chars */
+	cpi->parameter_value_length = 4*ETH_ALEN;
+
+	cfg_nwadr = (wchar_t *)((ulong)cpi + cpi->parameter_name_offset);
+	cfg_mac = (wchar_t *)((ulong)cpi + cpi->parameter_value_offset);
+	ret = utf8s_to_utf16s(NWADR_STR, NWADR_STRLEN, UTF16_HOST_ENDIAN,
+			      cfg_nwadr, NWADR_STRLEN);
+	if (ret < 0)
+		goto cleanup;
+	snprintf(macstr, 2*ETH_ALEN+1, "%pm", mac);
+	ret = utf8s_to_utf16s(macstr, 2*ETH_ALEN, UTF16_HOST_ENDIAN,
+			      cfg_mac, 2*ETH_ALEN);
+	if (ret < 0)
+		goto cleanup;
+
+	ret = rndis_filter_send_request(rdev, request);
+	if (ret != 0)
+		goto cleanup;
+
+	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+	if (t == 0) {
+		netdev_err(ndev, "timeout before we got a set response...\n");
+		/*
+		 * can't put_rndis_request, since we may still receive a
+		 * send-completion.
+		 */
+		return -EBUSY;
+	} else {
+		set_complete = &request->response_msg.msg.set_complete;
+		if (set_complete->status != RNDIS_STATUS_SUCCESS)
+			ret = -EINVAL;
+	}
+
+cleanup:
+	put_rndis_request(rdev, request);
+	return ret;
+}
+
+
 static int rndis_filter_query_device_link_status(struct rndis_device *dev)
 {
 	u32 size = sizeof(u32);
-- 
1.7.4.1