| Message ID | 1435868255-10865-1-git-send-email-haiyangz@microsoft.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Jul 02, 2015 at 01:17:35PM -0700, Haiyang Zhang wrote: > When packet encapsulation is in use, the MTU needs to be reduced for > headroom reservation. > The existing code takes the updated MTU value only from the host side. > But vSwitch extensions, such as Open vSwitch, require the flexibility > to change the MTU to different values from within a guest during the > lifecycle of a vNIC, when the encapsulation protocol is changed. The > patch supports this kind of MTU changes. > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > Reviewed-by: K. Y. Srinivasan <kys@microsoft.com> > --- > drivers/net/hyperv/netvsc_drv.c | 3 +-- > drivers/net/hyperv/rndis_filter.c | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 358475e..68e7ece 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu) > if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) > limit = NETVSC_MTU - ETH_HLEN; > > - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ > - if (mtu < ETH_DATA_LEN || mtu > limit) > + if (mtu < 68 || mtu > limit) How did you calculate 68? Avoid magic numbers like this, make it a define. regards, dan carpenter -- 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
From: Dan Carpenter <dan.carpenter@oracle.com> Date: Fri, 3 Jul 2015 14:28:47 +0300 > On Thu, Jul 02, 2015 at 01:17:35PM -0700, Haiyang Zhang wrote: >> When packet encapsulation is in use, the MTU needs to be reduced for >> headroom reservation. >> The existing code takes the updated MTU value only from the host side. >> But vSwitch extensions, such as Open vSwitch, require the flexibility >> to change the MTU to different values from within a guest during the >> lifecycle of a vNIC, when the encapsulation protocol is changed. The >> patch supports this kind of MTU changes. >> >> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> >> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com> >> --- >> drivers/net/hyperv/netvsc_drv.c | 3 +-- >> drivers/net/hyperv/rndis_filter.c | 2 +- >> 2 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c >> index 358475e..68e7ece 100644 >> --- a/drivers/net/hyperv/netvsc_drv.c >> +++ b/drivers/net/hyperv/netvsc_drv.c >> @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu) >> if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) >> limit = NETVSC_MTU - ETH_HLEN; >> >> - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ >> - if (mtu < ETH_DATA_LEN || mtu > limit) >> + if (mtu < 68 || mtu > limit) > > How did you calculate 68? Avoid magic numbers like this, make it a > define. Agreed. -- 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: David Miller [mailto:davem@davemloft.net] > Sent: Friday, July 3, 2015 12:17 PM > To: dan.carpenter@oracle.com > Cc: Haiyang Zhang; netdev@vger.kernel.org; olaf@aepfle.de; > jasowang@redhat.com; driverdev-devel@linuxdriverproject.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH net-next] hv_netvsc: Add support to set MTU > reservation from guest side > > From: Dan Carpenter <dan.carpenter@oracle.com> > Date: Fri, 3 Jul 2015 14:28:47 +0300 > > > On Thu, Jul 02, 2015 at 01:17:35PM -0700, Haiyang Zhang wrote: > >> When packet encapsulation is in use, the MTU needs to be reduced for > >> headroom reservation. > >> The existing code takes the updated MTU value only from the host side. > >> But vSwitch extensions, such as Open vSwitch, require the flexibility > >> to change the MTU to different values from within a guest during the > >> lifecycle of a vNIC, when the encapsulation protocol is changed. The > >> patch supports this kind of MTU changes. > >> > >> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > >> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com> > >> --- > >> drivers/net/hyperv/netvsc_drv.c | 3 +-- > >> drivers/net/hyperv/rndis_filter.c | 2 +- > >> 2 files changed, 2 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > >> index 358475e..68e7ece 100644 > >> --- a/drivers/net/hyperv/netvsc_drv.c > >> +++ b/drivers/net/hyperv/netvsc_drv.c > >> @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device > *ndev, int mtu) > >> if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) > >> limit = NETVSC_MTU - ETH_HLEN; > >> > >> - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ > >> - if (mtu < ETH_DATA_LEN || mtu > limit) > >> + if (mtu < 68 || mtu > limit) > > > > How did you calculate 68? Avoid magic numbers like this, make it a > > define. > > Agreed. The number 68 is same as the common code in eth_change_mtu(). I think it came from the protocol requirement: https://en.wikipedia.org/wiki/Maximum_transmission_unit Sure, I will put the number in a define, and resubmit the patch. Thanks, - Haiyang -- 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
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 358475e..68e7ece 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu) if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) limit = NETVSC_MTU - ETH_HLEN; - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ - if (mtu < ETH_DATA_LEN || mtu > limit) + if (mtu < 68 || mtu > limit) return -EINVAL; nvdev->start_remove = true; diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 006c1b8..172824e 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1053,7 +1053,7 @@ int rndis_filter_device_add(struct hv_device *dev, ret = rndis_filter_query_device(rndis_device, RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE, &mtu, &size); - if (ret == 0 && size == sizeof(u32)) + if (ret == 0 && size == sizeof(u32) && mtu < net_device->ndev->mtu) net_device->ndev->mtu = mtu; /* Get the mac address */