@@ -41,21 +41,6 @@ static int wil_stop(struct net_device *ndev)returnwil_down(wil);}-staticintwil_change_mtu(structnet_device*ndev,intnew_mtu)-{-structwil6210_priv*wil=ndev_to_wil(ndev);--if(new_mtu<68||new_mtu>mtu_max){-wil_err(wil,"invalid MTU %d\n",new_mtu);-return-EINVAL;-}--wil_dbg_misc(wil,"change MTU %d -> %d\n",ndev->mtu,new_mtu);-ndev->mtu=new_mtu;--return0;-}-staticintwil_do_ioctl(structnet_device*ndev,structifreq*ifr,intcmd){structwil6210_priv*wil=ndev_to_wil(ndev);
@@ -756,6 +743,11 @@ int wlan_setup(struct wlandevice *wlandev, struct device *physdev)wdev->wiphy=wiphy;wdev->iftype=NL80211_IFTYPE_STATION;netdev->ieee80211_ptr=wdev;+netdev->min_mtu=68;+/* 2312 is max 802.11 payload, 20 is overhead,+*(ether+llc+snap)andanother8forwep.+*/+netdev->max_mtu=(2312-20-8);netif_stop_queue(netdev);netif_carrier_off(netdev);
@@ -1980,11 +1980,6 @@ static int lan78xx_change_mtu(struct net_device *netdev, int new_mtu)intold_rx_urb_size=dev->rx_urb_size;intret;-if(new_mtu>MAX_SINGLE_PACKET_SIZE)-return-EINVAL;--if(new_mtu<=0)-return-EINVAL;/* no second zero-length packet read wanted after mtu-sized packets */if((ll_mtu%dev->maxpacket)==0)return-EDOM;
@@ -3388,6 +3383,9 @@ static int lan78xx_probe(struct usb_interface *intf,if(netdev->mtu>(dev->hard_mtu-netdev->hard_header_len))netdev->mtu=dev->hard_mtu-netdev->hard_header_len;+/* MTU range: 68 - 9000 */+netdev->max_mtu=MAX_SINGLE_PACKET_SIZE;+dev->ep_blkin=(intf->cur_altsetting)->endpoint+0;dev->ep_blkout=(intf->cur_altsetting)->endpoint+1;dev->ep_intr=(intf->cur_altsetting)->endpoint+2;
@@ -384,8 +384,6 @@ int usbnet_change_mtu (struct net_device *net, int new_mtu)intold_hard_mtu=dev->hard_mtu;intold_rx_urb_size=dev->rx_urb_size;-if(new_mtu<=0)-return-EINVAL;// no second zero-length packet read wanted after mtu-sized packetsif((ll_mtu%dev->maxpacket)==0)return-EDOM;
@@ -1096,6 +1095,8 @@ static int fr_add_pvc(struct net_device *frad, unsigned int dlci, int type)}dev->netdev_ops=&pvc_ops;dev->mtu=HDLC_MAX_MTU;+dev->min_mtu=68;+dev->max_mtu=HDLC_MAX_MTU;dev->priv_flags|=IFF_NO_QUEUE;dev->ml_priv=pvc;
@@ -93,8 +93,6 @@ static __inline__ void debug_frame(const struct sk_buff *skb)inthdlc_open(structnet_device*dev);/* Must be called by hardware driver when HDLC device is being closed */voidhdlc_close(structnet_device*dev);-/* May be used by hardware driver */-inthdlc_change_mtu(structnet_device*dev,intnew_mtu);/* Must be pointed to by hw driver's dev->netdev_ops->ndo_start_xmit */netdev_tx_thdlc_start_xmit(structsk_buff*skb,structnet_device*dev);
@@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,max_mtu=(1<<(card->max_receive+1))-sizeof(structrfc2734_header)-IEEE1394_GASP_HDR_SIZE;net->mtu=min(1500U,max_mtu);+net->min_mtu=ETH_MIN_MTU;+net->max_mtu=net->mtu;/* Set our hardware address while we're at it */ha=(unionfwnet_hwaddr*)net->dev_addr;
@@ -118,6 +118,8 @@ static DEFINE_SPINLOCK(xpnet_broadcast_lock);*now,thedefaultis64KB.*/#define XPNET_MAX_MTU (0x800000UL - L1_CACHE_BYTES)+/* 68 comes from min TCP+IP+MAC header */+#define XPNET_MIN_MTU 68/* 32KB has been determined to be the ideal */#define XPNET_DEF_MTU (0x8000UL)
@@ -330,22 +332,6 @@ xpnet_dev_stop(struct net_device *dev)return0;}-staticint-xpnet_dev_change_mtu(structnet_device*dev,intnew_mtu)-{-/* 68 comes from min TCP+IP+MAC header */-if((new_mtu<68)||(new_mtu>XPNET_MAX_MTU)){-dev_err(xpnet,"ifconfig %s mtu %d failed; value must be "-"between 68 and %ld\n",dev->name,new_mtu,-XPNET_MAX_MTU);-return-EINVAL;-}--dev->mtu=new_mtu;-dev_dbg(xpnet,"ifconfig %s mtu set to %d\n",dev->name,new_mtu);-return0;-}-/**Notificationthattheotherendhasreceivedthemessageand*DMA'dtheskbinformation.Atthispoint,theyaredonewith
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev)dev->vlan_features=dev->features;+/* MTU range: 68 - 65535 */+dev->min_mtu=MIN_MTU;+dev->max_mtu=MAX_MTU;+/* Configuration may specify what MAC to use. Otherwise random. */if(virtio_has_feature(vdev,VIRTIO_NET_F_MAC))virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev)mtu=virtio_cread16(vdev,offsetof(structvirtio_net_config,mtu));-if(virtnet_change_mtu(dev,mtu))+if(mtu>=dev->min_mtu&&mtu<=dev->max_mtu){+dev->mtu=mtu;__virtio_clear_bit(vdev,VIRTIO_NET_F_MTU);+}}if(vi->any_header_sg)
From: Jarod Wilson <hidden> Date: 2016-10-19 02:35:26
geneve:
- Merge __geneve_change_mtu back into geneve_change_mtu, set max_mtu
- This one isn't quite as straight-forward as others, could use some
closer inspection and testing
macvlan:
- set min/max_mtu
tun:
- set min/max_mtu, remove tun_net_change_mtu
vxlan:
- Merge __vxlan_change_mtu back into vxlan_change_mtu, set min/max_mtu
- This one is also not as straight-forward and could use closer inspection
and testing from vxlan folks
bridge:
- set max_mtu via br_min_mtu()
openvswitch:
- set min/max_mtu, remove internal_dev_change_mtu
- note: max_mtu wasn't checked previously, it's been set to 65535, which
is the largest possible size supported
sch_teql:
- set min/max_mtu (note: max_mtu previously unchecked, used max of 65535)
CC: netdev@vger.kernel.org
CC: Nicolas Dichtel <redacted>
CC: Hannes Frederic Sowa <redacted>
CC: Tom Herbert <redacted>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Alexander Duyck <redacted>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Jiri Benc <redacted>
CC: WANG Cong <redacted>
CC: Roopa Prabhu <redacted>
CC: Pravin B Shelar <redacted>
CC: Sabrina Dubroca <sd@queasysnail.net>
CC: Patrick McHardy <redacted>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: Pravin Shelar <redacted>
Signed-off-by: Jarod Wilson <redacted>
---
drivers/net/geneve.c | 48 +++++++++++-----------------
drivers/net/macvlan.c | 6 +++-
drivers/net/tun.c | 20 ++++--------
drivers/net/vxlan.c | 62 ++++++++++++++++++------------------
net/bridge/br_device.c | 9 +++---
net/openvswitch/vport-internal_dev.c | 10 ------
net/sched/sch_teql.c | 5 ++-
7 files changed, 67 insertions(+), 93 deletions(-)
@@ -1034,39 +1034,18 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)returngeneve_xmit_skb(skb,dev,info);}-staticint__geneve_change_mtu(structnet_device*dev,intnew_mtu,boolstrict)+staticintgeneve_change_mtu(structnet_device*dev,intnew_mtu){-structgeneve_dev*geneve=netdev_priv(dev);-/* The max_mtu calculation does not take account of GENEVE-*options,toavoidexcludingpotentiallyvalid-*configurations.+/* Only possible if called internally, ndo_change_mtu path's new_mtu+*isguaranteedtobebetweendev->min_mtuanddev->max_mtu.*/-intmax_mtu=IP_MAX_MTU-GENEVE_BASE_HLEN-dev->hard_header_len;--if(geneve->remote.sa.sa_family==AF_INET6)-max_mtu-=sizeof(structipv6hdr);-else-max_mtu-=sizeof(structiphdr);--if(new_mtu<68)-return-EINVAL;--if(new_mtu>max_mtu){-if(strict)-return-EINVAL;--new_mtu=max_mtu;-}+if(new_mtu>dev->max_mtu)+new_mtu=dev->max_mtu;dev->mtu=new_mtu;return0;}-staticintgeneve_change_mtu(structnet_device*dev,intnew_mtu)-{-return__geneve_change_mtu(dev,new_mtu,true);-}-staticintgeneve_fill_metadata_dst(structnet_device*dev,structsk_buff*skb){structip_tunnel_info*info=skb_tunnel_info(skb);
@@ -1170,6 +1149,14 @@ static void geneve_setup(struct net_device *dev)dev->hw_features|=NETIF_F_SG|NETIF_F_HW_CSUM|NETIF_F_RXCSUM;dev->hw_features|=NETIF_F_GSO_SOFTWARE;+/* MTU range: 68 - (something less than 65535) */+dev->min_mtu=ETH_MIN_MTU;+/* The max_mtu calculation does not take account of GENEVE+*options,toavoidexcludingpotentiallyvalid+*configurations.ThiswillbefurtherreducedbyIPvXhdrsize.+*/+dev->max_mtu=IP_MAX_MTU-GENEVE_BASE_HLEN-dev->hard_header_len;+netif_keep_dst(dev);dev->priv_flags&=~IFF_TX_SKB_SHARING;dev->priv_flags|=IFF_LIVE_ADDR_CHANGE|IFF_NO_QUEUE;
@@ -1285,10 +1272,13 @@ static int geneve_configure(struct net *net, struct net_device *dev,/* make enough headroom for basic scenario */encap_len=GENEVE_BASE_HLEN+ETH_HLEN;-if(remote->sa.sa_family==AF_INET)+if(remote->sa.sa_family==AF_INET){encap_len+=sizeof(structiphdr);-else+dev->max_mtu-=sizeof(structiphdr);+}else{encap_len+=sizeof(structipv6hdr);+dev->max_mtu-=sizeof(structipv6hdr);+}dev->needed_headroom=encap_len+ETH_HLEN;if(metadata){
@@ -1488,7 +1478,7 @@ struct net_device *geneve_dev_create_fb(struct net *net, const char *name,/* openvswitch users expect packet sizes to be unrestricted,*sosetthelargestMTUwecan.*/-err=__geneve_change_mtu(dev,IP_MAX_MTU,false);+err=geneve_change_mtu(dev,IP_MAX_MTU);if(err)gotoerr;
@@ -2367,43 +2367,31 @@ static void vxlan_set_multicast_list(struct net_device *dev){}-staticint__vxlan_change_mtu(structnet_device*dev,-structnet_device*lowerdev,-structvxlan_rdst*dst,intnew_mtu,boolstrict)+staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu){-intmax_mtu=IP_MAX_MTU;--if(lowerdev)-max_mtu=lowerdev->mtu;+structvxlan_dev*vxlan=netdev_priv(dev);+structvxlan_rdst*dst=&vxlan->default_dst;+structnet_device*lowerdev=__dev_get_by_index(vxlan->net,+dst->remote_ifindex);+booluse_ipv6=false;if(dst->remote_ip.sa.sa_family==AF_INET6)-max_mtu-=VXLAN6_HEADROOM;-else-max_mtu-=VXLAN_HEADROOM;--if(new_mtu<68)-return-EINVAL;+use_ipv6=true;-if(new_mtu>max_mtu){-if(strict)+/* We re-check this, because users *could* alter the mtu of the+*lowerdeviceafterwe'veinitializeddev->max_mtu.+*/+if(lowerdev){+dev->max_mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+if(new_mtu>dev->max_mtu)return-EINVAL;--new_mtu=max_mtu;}dev->mtu=new_mtu;return0;}-staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu)-{-structvxlan_dev*vxlan=netdev_priv(dev);-structvxlan_rdst*dst=&vxlan->default_dst;-structnet_device*lowerdev=__dev_get_by_index(vxlan->net,-dst->remote_ifindex);-return__vxlan_change_mtu(dev,lowerdev,dst,new_mtu,true);-}-staticintvxlan_fill_metadata_dst(structnet_device*dev,structsk_buff*skb){structvxlan_dev*vxlan=netdev_priv(dev);
@@ -2795,6 +2783,10 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,vxlan_ether_setup(dev);}+/* MTU range: 68 - 65535 */+dev->min_mtu=68;+dev->max_mtu=IP_MAX_MTU;+vxlan->net=src_net;dst->remote_vni=conf->vni;
@@ -2837,8 +2829,11 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,}#endif-if(!conf->mtu)-dev->mtu=lowerdev->mtu-(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+if(!conf->mtu){+dev->mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+dev->max_mtu=dev->mtu;+}needed_headroom=lowerdev->hard_header_len;}elseif(vxlan_addr_multicast(&dst->remote_ip)){
@@ -2847,9 +2842,14 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,}if(conf->mtu){-err=__vxlan_change_mtu(dev,lowerdev,dst,conf->mtu,false);-if(err)-returnerr;+if(lowerdev)+dev->max_mtu=lowerdev->mtu;+dev->max_mtu-=(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);++dev->mtu=conf->mtu;++if(conf->mtu>dev->max_mtu)+dev->mtu=dev->max_mtu;}if(use_ipv6||conf->flags&VXLAN_F_COLLECT_METADATA)
@@ -184,17 +184,15 @@ static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,staticintbr_change_mtu(structnet_device*dev,intnew_mtu){+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)structnet_bridge*br=netdev_priv(dev);-if(new_mtu<68||new_mtu>br_min_mtu(br))-return-EINVAL;--dev->mtu=new_mtu;-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)/* remember the MTU in the rtable for PMTU */dst_metric_set(&br->fake_rtable.dst,RTAX_MTU,new_mtu);#endif+dev->mtu=new_mtu;+return0;}
geneve:
- Merge __geneve_change_mtu back into geneve_change_mtu, set max_mtu
- This one isn't quite as straight-forward as others, could use some
closer inspection and testing
macvlan:
- set min/max_mtu
tun:
- set min/max_mtu, remove tun_net_change_mtu
vxlan:
- Merge __vxlan_change_mtu back into vxlan_change_mtu, set min/max_mtu
- This one is also not as straight-forward and could use closer inspection
and testing from vxlan folks
bridge:
- set max_mtu via br_min_mtu()
openvswitch:
- set min/max_mtu, remove internal_dev_change_mtu
- note: max_mtu wasn't checked previously, it's been set to 65535, which
is the largest possible size supported
sch_teql:
- set min/max_mtu (note: max_mtu previously unchecked, used max of 65535)
Nothing for other virtual netdevices? (dummy, veth, bond, etc) Their
MTU is limited to 1500 now. Also missing macsec and ip_gre, probably
others that are using ether_setup.
[...]
@@ -184,17 +184,15 @@ static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,staticintbr_change_mtu(structnet_device*dev,intnew_mtu){+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)structnet_bridge*br=netdev_priv(dev);-if(new_mtu<68||new_mtu>br_min_mtu(br))-return-EINVAL;--dev->mtu=new_mtu;-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)/* remember the MTU in the rtable for PMTU */dst_metric_set(&br->fake_rtable.dst,RTAX_MTU,new_mtu);#endif+dev->mtu=new_mtu;+return0;}
br_min_mtu uses br->port_list, which is only initialized a few lines
later (right after the spin_lock_init() at the end of the context of
this diff).
Besides, I don't think this works: br_min_mtu(br) changes when you add
and remove ports, or when you change the MTU of an enslaved
device. But this makes the max MTU for the bridge fixed (to 1500).
Probably. That value crops up in multiple drivers.
quoted
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev) dev->vlan_features = dev->features;+ /* MTU range: 68 - 65535 */+ dev->min_mtu = MIN_MTU;+ dev->max_mtu = MAX_MTU;+ /* Configuration may specify what MAC to use. Otherwise random. */ if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev) mtu = virtio_cread16(vdev, offsetof(struct virtio_net_config, mtu));- if (virtnet_change_mtu(dev, mtu))+ if (mtu >= dev->min_mtu && mtu <= dev->max_mtu) {+ dev->mtu = mtu; __virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
I think the logic is wrong here:
If mtu is legal, we set it but do not tell host.
If it's out of range we tell host we use it
but don't actually.
Should be the reverse.
Ah, yes, looks like it should be:
if (mtu < dev->min_mtu || mtu > dev->max_mtu)
__virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
else
dev->mtu = mtu;
--
Jarod Wilson
jarod@redhat.com
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev) dev->vlan_features = dev->features;+ /* MTU range: 68 - 65535 */+ dev->min_mtu = MIN_MTU;+ dev->max_mtu = MAX_MTU;+ /* Configuration may specify what MAC to use. Otherwise random. */ if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev) mtu = virtio_cread16(vdev, offsetof(struct virtio_net_config, mtu));- if (virtnet_change_mtu(dev, mtu))+ if (mtu >= dev->min_mtu && mtu <= dev->max_mtu) {+ dev->mtu = mtu; __virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
I think the logic is wrong here:
If mtu is legal, we set it but do not tell host.
If it's out of range we tell host we use it
but don't actually.
Should be the reverse.
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev) dev->vlan_features = dev->features;+ /* MTU range: 68 - 65535 */+ dev->min_mtu = MIN_MTU;+ dev->max_mtu = MAX_MTU;+ /* Configuration may specify what MAC to use. Otherwise random. */ if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev) mtu = virtio_cread16(vdev, offsetof(struct virtio_net_config, mtu));- if (virtnet_change_mtu(dev, mtu))+ if (mtu >= dev->min_mtu && mtu <= dev->max_mtu) {+ dev->mtu = mtu; __virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
I think the logic is wrong here:
If mtu is legal, we set it but do not tell host.
If it's out of range we tell host we use it
but don't actually.
Should be the reverse.
Why change it to 65535? For Hyperv host, this should be 65536.
Forgot to call this change out, sorry. That was changed, because of
IP_MAX_MTU being 0xFFFFU -> 65535.
quoted
@@ -1343,6 +1336,13 @@ static int netvsc_probe(struct hv_device *dev, netif_carrier_off(net);+ /* MTU range: 68 - 1500 or 65521 */+ net->min_mtu = NETVSC_MTU_MIN;+ if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)+ net->max_mtu = NETVSC_MTU - ETH_HLEN;+ else+ net->max_mtu = ETH_DATA_LEN;+ netvsc_init_settings(net); net_device_ctx = netdev_priv(net);
nvdev->nvsp_version is not set until after rndis_filter_device_add()
is successfully completed.
You need to move this part to the place just before this line:
ret = register_netdev(net);
Okay, will fix that up.
--
Jarod Wilson
jarod@redhat.com
CC: Jes Sorensen <redacted>
CC: Marek Lindner <redacted>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Antonio Quartulli <redacted>
Signed-off-by: Jarod Wilson <redacted>
From: Jarod Wilson <hidden> Date: 2016-10-19 14:40:18
On Wed, Oct 19, 2016 at 03:55:29PM +0200, Sabrina Dubroca wrote:
2016-10-18, 22:33:31 -0400, Jarod Wilson wrote:
quoted
geneve:
- Merge __geneve_change_mtu back into geneve_change_mtu, set max_mtu
- This one isn't quite as straight-forward as others, could use some
closer inspection and testing
macvlan:
- set min/max_mtu
tun:
- set min/max_mtu, remove tun_net_change_mtu
vxlan:
- Merge __vxlan_change_mtu back into vxlan_change_mtu, set min/max_mtu
- This one is also not as straight-forward and could use closer inspection
and testing from vxlan folks
bridge:
- set max_mtu via br_min_mtu()
openvswitch:
- set min/max_mtu, remove internal_dev_change_mtu
- note: max_mtu wasn't checked previously, it's been set to 65535, which
is the largest possible size supported
sch_teql:
- set min/max_mtu (note: max_mtu previously unchecked, used max of 65535)
Nothing for other virtual netdevices? (dummy, veth, bond, etc) Their
MTU is limited to 1500 now. Also missing macsec and ip_gre, probably
others that are using ether_setup.
Yeah, I've clearly missed more than I thought. Doing another sweep now.
I'm thinking more and more that we ought to back out the patch that sets
min/max in ether_setup, save it for last, after we're sure everyone that
calls it has been prepared.
@@ -184,17 +184,15 @@ static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,staticintbr_change_mtu(structnet_device*dev,intnew_mtu){+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)structnet_bridge*br=netdev_priv(dev);-if(new_mtu<68||new_mtu>br_min_mtu(br))-return-EINVAL;--dev->mtu=new_mtu;-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)/* remember the MTU in the rtable for PMTU */dst_metric_set(&br->fake_rtable.dst,RTAX_MTU,new_mtu);#endif+dev->mtu=new_mtu;+return0;}
br_min_mtu uses br->port_list, which is only initialized a few lines
later (right after the spin_lock_init() at the end of the context of
this diff).
Ah, okay, I'd just grouped it with the other dev->foo settings.
Besides, I don't think this works: br_min_mtu(br) changes when you add
and remove ports, or when you change the MTU of an enslaved
device. But this makes the max MTU for the bridge fixed (to 1500).
Okay, how about this: set no max_mtu (or set it to IP_MAX_MTU/65535), and
then retain a check against the possibly ever-changing br_min_mtu(br) in
br_change_mtu()?
@@ -2367,43 +2367,31 @@ static void vxlan_set_multicast_list(struct net_device *dev){}-staticint__vxlan_change_mtu(structnet_device*dev,-structnet_device*lowerdev,-structvxlan_rdst*dst,intnew_mtu,boolstrict)+staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu){-intmax_mtu=IP_MAX_MTU;--if(lowerdev)-max_mtu=lowerdev->mtu;+structvxlan_dev*vxlan=netdev_priv(dev);+structvxlan_rdst*dst=&vxlan->default_dst;+structnet_device*lowerdev=__dev_get_by_index(vxlan->net,+dst->remote_ifindex);+booluse_ipv6=false;if(dst->remote_ip.sa.sa_family==AF_INET6)-max_mtu-=VXLAN6_HEADROOM;-else-max_mtu-=VXLAN_HEADROOM;--if(new_mtu<68)-return-EINVAL;+use_ipv6=true;-if(new_mtu>max_mtu){-if(strict)+/* We re-check this, because users *could* alter the mtu of the+*lowerdeviceafterwe'veinitializeddev->max_mtu.+*/+if(lowerdev){+dev->max_mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+if(new_mtu>dev->max_mtu)return-EINVAL;--new_mtu=max_mtu;}dev->mtu=new_mtu;return0;}
Sorry for the silly question, how does the min_mtu and max_mtu stuff
works? I noticed your patches but haven't looked in depth into them.
When the ndo_change_mtu callback is defined, is the dev->min_mtu and
dev->max_mtu checked first and if the desired mtu is not within range,
ndo_change_mtu is not called?
Or does ndo_change_mtu override the checks?
In either case, the code does not look correct. In the first case,
increasing of lowerdev MTU wouldn't allow increasing of vxlan MTU
without deleting and recreating the vxlan interface. In the second
case, you're missing check against the min_mtu.
@@ -2367,43 +2367,31 @@ static void vxlan_set_multicast_list(struct net_device *dev){}-staticint__vxlan_change_mtu(structnet_device*dev,-structnet_device*lowerdev,-structvxlan_rdst*dst,intnew_mtu,boolstrict)+staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu){-intmax_mtu=IP_MAX_MTU;--if(lowerdev)-max_mtu=lowerdev->mtu;+structvxlan_dev*vxlan=netdev_priv(dev);+structvxlan_rdst*dst=&vxlan->default_dst;+structnet_device*lowerdev=__dev_get_by_index(vxlan->net,+dst->remote_ifindex);+booluse_ipv6=false;if(dst->remote_ip.sa.sa_family==AF_INET6)-max_mtu-=VXLAN6_HEADROOM;-else-max_mtu-=VXLAN_HEADROOM;--if(new_mtu<68)-return-EINVAL;+use_ipv6=true;-if(new_mtu>max_mtu){-if(strict)+/* We re-check this, because users *could* alter the mtu of the+*lowerdeviceafterwe'veinitializeddev->max_mtu.+*/+if(lowerdev){+dev->max_mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+if(new_mtu>dev->max_mtu)return-EINVAL;--new_mtu=max_mtu;}dev->mtu=new_mtu;return0;}
Sorry for the silly question, how does the min_mtu and max_mtu stuff
works? I noticed your patches but haven't looked in depth into them.
When the ndo_change_mtu callback is defined, is the dev->min_mtu and
dev->max_mtu checked first and if the desired mtu is not within range,
ndo_change_mtu is not called?
Or does ndo_change_mtu override the checks?
The former. If the new value is outside min/max, ndo_change_mtu doesn't
get called, which is exactly the chicken and egg problem I introduced by
setting max_mtu to 1500 in ether_setup before having all drivers that call
ether_setup set a more appropriate max_mtu first. :\
In either case, the code does not look correct. In the first case,
increasing of lowerdev MTU wouldn't allow increasing of vxlan MTU
without deleting and recreating the vxlan interface. In the second
case, you're missing check against the min_mtu.
Okay, this sounds like a similar case to bridge that Sabrina pointed out.
Looks like virtual devices will need to just set no max_mtu directly (or
IP_MAX_MTU), and do dynamic checks in their ndo_change_mtu if they need to
compare against underlying devices on the fly.
...
quoted
@@ -2847,9 +2842,14 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev, } if (conf->mtu) {- err = __vxlan_change_mtu(dev, lowerdev, dst, conf->mtu, false);- if (err)- return err;+ if (lowerdev)+ dev->max_mtu = lowerdev->mtu;+ dev->max_mtu -= (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM);++ dev->mtu = conf->mtu;++ if (conf->mtu > dev->max_mtu)+ dev->mtu = dev->max_mtu; }
You removed the check for min_mtu but it's needed here. The conf->mtu
value comes from the user space and can be anything.
Hm. Not sure why I did that... Will put it back now...
--
Jarod Wilson
jarod@redhat.com
On Wed, Oct 19, 2016 at 03:55:29PM +0200, Sabrina Dubroca wrote:
quoted
2016-10-18, 22:33:31 -0400, Jarod Wilson wrote:
quoted
geneve:
- Merge __geneve_change_mtu back into geneve_change_mtu, set max_mtu
- This one isn't quite as straight-forward as others, could use some
closer inspection and testing
macvlan:
- set min/max_mtu
tun:
- set min/max_mtu, remove tun_net_change_mtu
vxlan:
- Merge __vxlan_change_mtu back into vxlan_change_mtu, set min/max_mtu
- This one is also not as straight-forward and could use closer inspection
and testing from vxlan folks
bridge:
- set max_mtu via br_min_mtu()
openvswitch:
- set min/max_mtu, remove internal_dev_change_mtu
- note: max_mtu wasn't checked previously, it's been set to 65535, which
is the largest possible size supported
sch_teql:
- set min/max_mtu (note: max_mtu previously unchecked, used max of 65535)
Nothing for other virtual netdevices? (dummy, veth, bond, etc) Their
MTU is limited to 1500 now. Also missing macsec and ip_gre, probably
others that are using ether_setup.
Yeah, I've clearly missed more than I thought. Doing another sweep now.
Thanks.
I'm thinking more and more that we ought to back out the patch that sets
min/max in ether_setup, save it for last, after we're sure everyone that
calls it has been prepared.
I'm not sure how that would work now, if some of the patches that
already went in for ethernet drivers assume that ether_setup will
configure a basic {min,max}_mtu pair (at least e100 makes that
assumption, but that might be the only one).
@@ -184,17 +184,15 @@ static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,staticintbr_change_mtu(structnet_device*dev,intnew_mtu){+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)structnet_bridge*br=netdev_priv(dev);-if(new_mtu<68||new_mtu>br_min_mtu(br))-return-EINVAL;--dev->mtu=new_mtu;-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)/* remember the MTU in the rtable for PMTU */dst_metric_set(&br->fake_rtable.dst,RTAX_MTU,new_mtu);#endif+dev->mtu=new_mtu;+return0;}
br_min_mtu uses br->port_list, which is only initialized a few lines
later (right after the spin_lock_init() at the end of the context of
this diff).
Ah, okay, I'd just grouped it with the other dev->foo settings.
quoted
Besides, I don't think this works: br_min_mtu(br) changes when you add
and remove ports, or when you change the MTU of an enslaved
device. But this makes the max MTU for the bridge fixed (to 1500).
Okay, how about this: set no max_mtu (or set it to IP_MAX_MTU/65535), and
then retain a check against the possibly ever-changing br_min_mtu(br) in
br_change_mtu()?
Why change it to 65535? For Hyperv host, this should be 65536.
quoted hunk
@@ -1343,6 +1336,13 @@ static int netvsc_probe(struct hv_device *dev, netif_carrier_off(net);+ /* MTU range: 68 - 1500 or 65521 */+ net->min_mtu = NETVSC_MTU_MIN;+ if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)+ net->max_mtu = NETVSC_MTU - ETH_HLEN;+ else+ net->max_mtu = ETH_DATA_LEN;+ netvsc_init_settings(net); net_device_ctx = netdev_priv(net);
nvdev->nvsp_version is not set until after rndis_filter_device_add()
is successfully completed.
You need to move this part to the place just before this line:
ret = register_netdev(net);
Thanks,
- Haiyang
From: Jarod Wilson <hidden> Date: 2016-10-19 15:46:58
On Wed, Oct 19, 2016 at 05:28:00PM +0200, Sabrina Dubroca wrote:
2016-10-19, 10:40:06 -0400, Jarod Wilson wrote:
quoted
On Wed, Oct 19, 2016 at 03:55:29PM +0200, Sabrina Dubroca wrote:
quoted
2016-10-18, 22:33:31 -0400, Jarod Wilson wrote:
...
quoted
I'm thinking more and more that we ought to back out the patch that sets
min/max in ether_setup, save it for last, after we're sure everyone that
calls it has been prepared.
I'm not sure how that would work now, if some of the patches that
already went in for ethernet drivers assume that ether_setup will
configure a basic {min,max}_mtu pair (at least e100 makes that
assumption, but that might be the only one).
Argh. Yeah. Hrm. Would have to do the revert *and* have e100 and possibly
others set their own min/max pair. So I guess it's a race to fix all the
fallout... Crap.
@@ -184,17 +184,15 @@ static struct rtnl_link_stats64 *br_get_stats64(struct net_device *dev,staticintbr_change_mtu(structnet_device*dev,intnew_mtu){+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)structnet_bridge*br=netdev_priv(dev);-if(new_mtu<68||new_mtu>br_min_mtu(br))-return-EINVAL;--dev->mtu=new_mtu;-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)/* remember the MTU in the rtable for PMTU */dst_metric_set(&br->fake_rtable.dst,RTAX_MTU,new_mtu);#endif+dev->mtu=new_mtu;+return0;}
br_min_mtu uses br->port_list, which is only initialized a few lines
later (right after the spin_lock_init() at the end of the context of
this diff).
Ah, okay, I'd just grouped it with the other dev->foo settings.
quoted
Besides, I don't think this works: br_min_mtu(br) changes when you add
and remove ports, or when you change the MTU of an enslaved
device. But this makes the max MTU for the bridge fixed (to 1500).
Okay, how about this: set no max_mtu (or set it to IP_MAX_MTU/65535), and
then retain a check against the possibly ever-changing br_min_mtu(br) in
br_change_mtu()?
Sounds good to me.
I think I have something here locally that looks sane. Working on a few
other similar cases now.
--
Jarod Wilson
jarod@redhat.com
From: Jarod Wilson <hidden> Date: 2016-10-19 19:29:11
On Wed, Oct 19, 2016 at 03:10:05PM -0400, David Miller wrote:
From: Jarod Wilson <redacted>
Date: Tue, 18 Oct 2016 22:33:27 -0400
quoted
This stack of patches should get absolutely everything in the kernel
converted from doing their own MTU range checking to the core MTU range
checking.
I'm expecting a respin of this series.
Absolutely. Will address all review comments, and have some additional
patches to add to the series to hopefully have absolutely everything
functional, asap.
--
Jarod Wilson
jarod@redhat.com
I need to check more closely, but I think the RFC 2734 encapsulation spec
and our implementation do not impose a particular upper limit. Though I
guess it's bad to let userland set arbitrarily large values here.
But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
PS:
If the IP packet plus encapsulation header fits into IEEE 1394 packet
payload, it is transported without link fragmentation. If it does not
fit, link fragmentation occurs (which reduces bandwidth a bit and
consumes additional buffering resources at the transmitter and the
receiver).
Broadcast and multicast packets are transmitted via IEEE 1394 asynchronous
stream packets at a low bus speed (because our code does not attempt to
find the maximum speed and size that is supported by all potential
listeners). This limits the payload to 512 bytes.
Unicast packets are transmitted via IEEE 1394 asynchronous write request
packets at optimum speed. In most cases, this means that 2048 bytes
payload is possible, in some cases 4096 bytes. Many CardBus FireWire
cards support only 1024 bytes payload of these packets though.
Furthermore, some low-speed long-haul cablings may cap the bus speed and
thereby the payload size to 1024 or 512 bytes, but this is uncommon in
practice.
--
Stefan Richter
-======----- =-=- =-=--
http://arcgraph.de/sr/
I need to check more closely, but I think the RFC 2734 encapsulation spec
and our implementation do not impose a particular upper limit. Though I
guess it's bad to let userland set arbitrarily large values here.
In which case, that would suggest using IP_MAX_MTU (65535) here.
But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
However, there's an explicit calculation for 'max_mtu' right there that I
glazed right over. It would seem perhaps *that* should be used for
net->max_mtu here, no?
PS:
If the IP packet plus encapsulation header fits into IEEE 1394 packet
payload, it is transported without link fragmentation. If it does not
fit, link fragmentation occurs (which reduces bandwidth a bit and
consumes additional buffering resources at the transmitter and the
receiver).
Broadcast and multicast packets are transmitted via IEEE 1394 asynchronous
stream packets at a low bus speed (because our code does not attempt to
find the maximum speed and size that is supported by all potential
listeners). This limits the payload to 512 bytes.
Unicast packets are transmitted via IEEE 1394 asynchronous write request
packets at optimum speed. In most cases, this means that 2048 bytes
payload is possible, in some cases 4096 bytes. Many CardBus FireWire
cards support only 1024 bytes payload of these packets though.
Furthermore, some low-speed long-haul cablings may cap the bus speed and
thereby the payload size to 1024 or 512 bytes, but this is uncommon in
practice.
Thorough as always, Stefan! :)
--
Jarod Wilson
jarod@redhat.com
@@ -103,13 +103,6 @@ static int ena_change_mtu(struct net_device *dev, int new_mtu)structena_adapter*adapter=netdev_priv(dev);intret;-if((new_mtu>adapter->max_mtu)||(new_mtu<ENA_MIN_MTU)){-netif_err(adapter,drv,dev,-"Invalid MTU setting. new_mtu: %d\n",new_mtu);--return-EINVAL;-}-ret=ena_com_set_dev_mtu(adapter->ena_dev,new_mtu);if(!ret){netif_dbg(adapter,drv,dev,"set MTU to %d\n",new_mtu);
@@ -2147,15 +2147,6 @@ static void sbmac_setmulti(struct sbmac_softc *sc)}}-staticintsb1250_change_mtu(structnet_device*_dev,intnew_mtu)-{-if(new_mtu>ENET_PACKET_SIZE)-return-EINVAL;-_dev->mtu=new_mtu;-pr_info("changing the mtu to %d\n",new_mtu);-return0;-}-staticconststructnet_device_opssbmac_netdev_ops={.ndo_open=sbmac_open,.ndo_stop=sbmac_close,
@@ -2229,6 +2219,8 @@ static int sbmac_init(struct platform_device *pldev, long long base)dev->netdev_ops=&sbmac_netdev_ops;dev->watchdog_timeo=TX_TIMEOUT;+dev->max_mtu=0;+dev->max_mtu=ENET_PACKET_SIZE;netif_napi_add(dev,&sc->napi,sbmac_poll,16);
@@ -1406,23 +1406,6 @@ static netdev_tx_t be_xmit(struct sk_buff *skb, struct net_device *netdev)returnNETDEV_TX_OK;}-staticintbe_change_mtu(structnet_device*netdev,intnew_mtu)-{-structbe_adapter*adapter=netdev_priv(netdev);-structdevice*dev=&adapter->pdev->dev;--if(new_mtu<BE_MIN_MTU||new_mtu>BE_MAX_MTU){-dev_info(dev,"MTU must be between %d and %d bytes\n",-BE_MIN_MTU,BE_MAX_MTU);-return-EINVAL;-}--dev_info(dev,"MTU changed from %d to %d bytes\n",-netdev->mtu,new_mtu);-netdev->mtu=new_mtu;-return0;-}-staticinlineboolbe_in_all_promisc(structbe_adapter*adapter){return(adapter->if_flags&BE_IF_FLAGS_ALL_PROMISCUOUS)==
@@ -2284,6 +2284,9 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,NETIF_F_HW_VLAN_CTAG_FILTER|NETIF_F_HW_TC;dev->hw_features|=NETIF_F_HW_TC;+dev->min_mtu=0;+dev->max_mtu=ETH_MAX_MTU;+/* Each packet needs to have a Tx header (metadata) on top all other*headers.*/
@@ -994,6 +994,9 @@ static int mlxsw_sx_port_create(struct mlxsw_sx *mlxsw_sx, u8 local_port)dev->features|=NETIF_F_NETNS_LOCAL|NETIF_F_LLTX|NETIF_F_SG|NETIF_F_VLAN_CHALLENGED;+dev->min_mtu=0;+dev->max_mtu=ETH_MAX_MTU;+/* Each packet needs to have a Tx header (metadata) on top all other*headers.*/
@@ -2190,6 +2181,8 @@ static int ns83820_init_one(struct pci_dev *pci_dev,ndev->features|=NETIF_F_SG;ndev->features|=NETIF_F_IP_CSUM;+ndev->min_mtu=0;+#ifdef NS83820_VLAN_ACCEL_SUPPORT/* We also support hardware vlan acceleration */ndev->features|=NETIF_F_HW_VLAN_CTAG_TX|NETIF_F_HW_VLAN_CTAG_RX;
@@ -4788,6 +4788,13 @@ static int qlge_probe(struct pci_dev *pdev,ndev->ethtool_ops=&qlge_ethtool_ops;ndev->watchdog_timeo=10*HZ;+/* MTU range: this driver only supports 1500 or 9000, so this only+*filtersoutvaluesaboveorbelow,andwe'llrelyon+*qlge_change_mtutomakesureonly1500or9000areallowed+*/+ndev->min_mtu=ETH_DATA_LEN;+ndev->max_mtu=9000;+err=register_netdev(ndev);if(err){dev_err(&pdev->dev,"net device registration failed.\n");
@@ -239,15 +239,8 @@ static void emac_rx_mode_set(struct net_device *netdev)/* Change the Maximum Transfer Unit (MTU) */staticintemac_change_mtu(structnet_device*netdev,intnew_mtu){-unsignedintmax_frame=new_mtu+ETH_HLEN+ETH_FCS_LEN+VLAN_HLEN;structemac_adapter*adpt=netdev_priv(netdev);-if((max_frame<EMAC_MIN_ETH_FRAME_SIZE)||-(max_frame>EMAC_MAX_ETH_FRAME_SIZE)){-netdev_err(adpt->netdev,"error: invalid MTU setting\n");-return-EINVAL;-}-netif_info(adpt,hw,adpt->netdev,"changing MTU from %d to %d\n",netdev->mtu,new_mtu);
@@ -679,6 +672,12 @@ static int emac_probe(struct platform_device *pdev)netdev->vlan_features|=NETIF_F_SG|NETIF_F_HW_CSUM|NETIF_F_TSO|NETIF_F_TSO6;+/* MTU range: 46 - 9194 */+netdev->min_mtu=EMAC_MIN_ETH_FRAME_SIZE-+(ETH_HLEN+ETH_FCS_LEN+VLAN_HLEN);+netdev->max_mtu=EMAC_MAX_ETH_FRAME_SIZE-+(ETH_HLEN+ETH_FCS_LEN+VLAN_HLEN);+INIT_WORK(&adpt->work_thread,emac_work_thread);/* Initialize queues */
@@ -36,6 +36,7 @@#define ETH_FCS_LEN 4 /* Octets in the FCS */#define ETH_MIN_MTU 68 /* Min IPv4 MTU per RFC791 */+#define ETH_MAX_MTU 0xFFFFU /* 65535, same as IP_MAX_MTU *//**ThesearethedefinedEthernetProtocolID's.
@@ -1096,6 +1095,8 @@ static int fr_add_pvc(struct net_device *frad, unsigned int dlci, int type)}dev->netdev_ops=&pvc_ops;dev->mtu=HDLC_MAX_MTU;+dev->min_mtu=68;+dev->max_mtu=HDLC_MAX_MTU;dev->priv_flags|=IFF_NO_QUEUE;dev->ml_priv=pvc;
@@ -93,8 +93,6 @@ static __inline__ void debug_frame(const struct sk_buff *skb)inthdlc_open(structnet_device*dev);/* Must be called by hardware driver when HDLC device is being closed */voidhdlc_close(structnet_device*dev);-/* May be used by hardware driver */-inthdlc_change_mtu(structnet_device*dev,intnew_mtu);/* Must be pointed to by hw driver's dev->netdev_ops->ndo_start_xmit */netdev_tx_thdlc_start_xmit(structsk_buff*skb,structnet_device*dev);
@@ -4202,10 +4202,6 @@ int qeth_change_mtu(struct net_device *dev, int new_mtu)sprintf(dbf_text,"%8x",new_mtu);QETH_CARD_TEXT(card,4,dbf_text);-if(new_mtu<64)-return-EINVAL;-if(new_mtu>65535)-return-EINVAL;if((!qeth_is_supported(card,IPA_IP_FRAGMENTATION))&&(!qeth_mtu_is_valid(card,new_mtu)))return-EINVAL;
From: Jarod Wilson <hidden> Date: 2016-10-20 17:56:05
firewire-net:
- set min/max_mtu
- remove fwnet_change_mtu
nes:
- set max_mtu
- clean up nes_netdev_change_mtu
xpnet:
- set min/max_mtu
- remove xpnet_dev_change_mtu
hippi:
- set min/max_mtu
- remove hippi_change_mtu
batman-adv:
- set max_mtu
- remove batadv_interface_change_mtu
- initialization is a little async, not 100% certain that max_mtu is set
in the optimal place, don't have hardware to test with
rionet:
- set min/max_mtu
- remove rionet_change_mtu
slip:
- set min/max_mtu
- streamline sl_change_mtu
um/net_kern:
- remove pointless ndo_change_mtu
hsi/clients/ssi_protocol:
- use core MTU range checking
- remove now redundant ssip_pn_set_mtu
ipoib:
- set a default max MTU value
- Note: ipoib's actual max MTU can vary, depending on if the device is in
connected mode or not, so we'll just set the max_mtu value to the max
possible, and let the ndo_change_mtu function continue to validate any new
MTU change requests with checks for CM or not. Note that ipoib has no
min_mtu set, and thus, the network core's mtu > 0 check is the only lower
bounds here.
mptlan:
- use net core MTU range checking
- remove now redundant mpt_lan_change_mtu
fddi:
- min_mtu = 21, max_mtu = 4470
- remove now redundant fddi_change_mtu (including export)
fjes:
- min_mtu = 8192, max_mtu = 65536
- The max_mtu value is actually one over IP_MAX_MTU here, but the idea is to
get past the core net MTU range checks so fjes_change_mtu can validate a
new MTU against what it supports (see fjes_support_mtu in fjes_hw.c)
hsr:
- min_mtu = 0 (calls ether_setup, max_mtu is 1500)
f_phonet:
- min_mtu = 6, max_mtu = 65541
u_ether:
- min_mtu = 14, max_mtu = 15412
phonet/pep-gprs:
- min_mtu = 576, max_mtu = 65530
- remove redundant gprs_set_mtu
CC: netdev@vger.kernel.org
CC: linux-rdma@vger.kernel.org
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: Faisal Latif <redacted>
CC: linux-rdma@vger.kernel.org
CC: Cliff Whickman <redacted>
CC: Robin Holt <robinmholt@gmail.com>
CC: Jes Sorensen <redacted>
CC: Marek Lindner <redacted>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Antonio Quartulli <redacted>
CC: Sathya Prakash <sathya.prakash@broadcom.com>
CC: Chaitra P B <redacted>
CC: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
CC: MPT-FusionLinux.pdl@broadcom.com
CC: Sebastian Reichel <sre@kernel.org>
CC: Felipe Balbi <balbi@kernel.org>
CC: Arvid Brodin <redacted>
CC: Remi Denis-Courmont <courmisch@gmail.com>
Signed-off-by: Jarod Wilson <redacted>
---
arch/um/drivers/net_kern.c | 8 --------
drivers/firewire/net.c | 18 ++++--------------
drivers/hsi/clients/ssi_protocol.c | 14 ++++----------
drivers/infiniband/hw/nes/nes.c | 1 -
drivers/infiniband/hw/nes/nes.h | 4 ++--
drivers/infiniband/hw/nes/nes_nic.c | 10 +++-------
drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
drivers/message/fusion/mptlan.c | 15 ++++-----------
drivers/misc/sgi-xp/xpnet.c | 21 ++++-----------------
drivers/net/fddi/skfp/skfddi.c | 1 -
drivers/net/fjes/fjes_main.c | 2 ++
drivers/net/hippi/rrunner.c | 1 -
drivers/net/rionet.c | 15 +++------------
drivers/net/slip/slip.c | 11 +++++------
drivers/usb/gadget/function/f_phonet.c | 11 ++---------
drivers/usb/gadget/function/u_ether.c | 14 ++++----------
include/linux/fddidevice.h | 1 -
include/linux/hippidevice.h | 1 -
net/802/fddi.c | 11 ++---------
net/802/hippi.c | 14 ++------------
net/batman-adv/soft-interface.c | 13 +------------
net/hsr/hsr_device.c | 1 +
net/phonet/pep-gprs.c | 12 ++----------
23 files changed, 46 insertions(+), 154 deletions(-)
@@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit,structnet_device*net;boolallocated_netdev=false;structfwnet_device*dev;-unsignedmax_mtu;intret;unionfwnet_hwaddr*ha;
@@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit,*UsetheRFC2734default1500octetsorthemaximumpayload*asinitialMTU*/-max_mtu=(1<<(card->max_receive+1))--sizeof(structrfc2734_header)-IEEE1394_GASP_HDR_SIZE;-net->mtu=min(1500U,max_mtu);+net->max_mtu=(1<<(card->max_receive+1))+-sizeof(structrfc2734_header)-IEEE1394_GASP_HDR_SIZE;+net->mtu=min(1500U,net->max_mtu);+net->min_mtu=ETH_MIN_MTU;/* Set our hardware address while we're at it */ha=(unionfwnet_hwaddr*)net->dev_addr;
@@ -2017,6 +2017,7 @@ static struct net_device *ipoib_add_port(const char *format,/* MTU will be reset when mcast join happens */priv->dev->mtu=IPOIB_UD_MTU(priv->max_ib_mtu);priv->mcast_mtu=priv->admin_mtu=priv->dev->mtu;+priv->dev->max_mtu=IPOIB_CM_MTU;priv->dev->neigh_priv_len=sizeof(structipoib_neigh);
@@ -118,6 +118,8 @@ static DEFINE_SPINLOCK(xpnet_broadcast_lock);*now,thedefaultis64KB.*/#define XPNET_MAX_MTU (0x800000UL - L1_CACHE_BYTES)+/* 68 comes from min TCP+IP+MAC header */+#define XPNET_MIN_MTU 68/* 32KB has been determined to be the ideal */#define XPNET_DEF_MTU (0x8000UL)
@@ -330,22 +332,6 @@ xpnet_dev_stop(struct net_device *dev)return0;}-staticint-xpnet_dev_change_mtu(structnet_device*dev,intnew_mtu)-{-/* 68 comes from min TCP+IP+MAC header */-if((new_mtu<68)||(new_mtu>XPNET_MAX_MTU)){-dev_err(xpnet,"ifconfig %s mtu %d failed; value must be "-"between 68 and %ld\n",dev->name,new_mtu,-XPNET_MAX_MTU);-return-EINVAL;-}--dev->mtu=new_mtu;-dev_dbg(xpnet,"ifconfig %s mtu set to %d\n",dev->name,new_mtu);-return0;-}-/**Notificationthattheotherendhasreceivedthemessageand*DMA'dtheskbinformation.Atthispoint,theyaredonewith
@@ -1634,7 +1634,7 @@ int ip6_tnl_change_mtu(struct net_device *dev, int new_mtu)structip6_tnl*tnl=netdev_priv(dev);if(tnl->parms.proto==IPPROTO_IPIP){-if(new_mtu<68)+if(new_mtu<ETH_MIN_MTU)return-EINVAL;}else{if(new_mtu<IPV6_MIN_MTU)
@@ -872,19 +872,12 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)structnetvsc_device*nvdev=ndevctx->nvdev;structhv_device*hdev=ndevctx->device_ctx;structnetvsc_device_infodevice_info;-intlimit=ETH_DATA_LEN;u32num_chn;intret=0;if(ndevctx->start_remove||!nvdev||nvdev->destroy)return-ENODEV;-if(nvdev->nvsp_version>=NVSP_PROTOCOL_VERSION_2)-limit=NETVSC_MTU-ETH_HLEN;--if(mtu<NETVSC_MTU_MIN||mtu>limit)-return-EINVAL;-ret=netvsc_close(ndev);if(ret)gotoout;
@@ -1402,6 +1395,13 @@ static int netvsc_probe(struct hv_device *dev,netif_set_real_num_tx_queues(net,nvdev->num_chn);netif_set_real_num_rx_queues(net,nvdev->num_chn);+/* MTU range: 68 - 1500 or 65521 */+net->min_mtu=NETVSC_MTU_MIN;+if(nvdev->nvsp_version>=NVSP_PROTOCOL_VERSION_2)+net->max_mtu=NETVSC_MTU-ETH_HLEN;+else+net->max_mtu=ETH_DATA_LEN;+ret=register_netdev(net);if(ret!=0){pr_err("Unable to register netdev.\n");
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev)dev->vlan_features=dev->features;+/* MTU range: 68 - 65535 */+dev->min_mtu=MIN_MTU;+dev->max_mtu=MAX_MTU;+/* Configuration may specify what MAC to use. Otherwise random. */if(virtio_has_feature(vdev,VIRTIO_NET_F_MAC))virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev)mtu=virtio_cread16(vdev,offsetof(structvirtio_net_config,mtu));-if(virtnet_change_mtu(dev,mtu))+if(mtu<dev->min_mtu||mtu>dev->max_mtu)__virtio_clear_bit(vdev,VIRTIO_NET_F_MTU);+else+dev->mtu=mtu;}if(vi->any_header_sg)
@@ -113,12 +113,10 @@ enum net_types {};-#define ETH_HEADER_SIZE 14 /* size of ethernet header */-#define ETH_MIN_DATA_SIZE 46 /* minimum eth data size */-#define ETH_MIN_PACKET_SIZE (ETH_HEADER_SIZE + ETH_MIN_DATA_SIZE)+#define ETH_MIN_PACKET_SIZE (ETH_HLEN + ETH_MIN_DATA_SIZE)-#define ETH_MAX_MTU 16384 /* maximum data size */+#define VISOR_ETH_MAX_MTU 16384 /* maximum data size */#ifndef MAX_MACADDR_LEN#define MAX_MACADDR_LEN 6 /* number of bytes in MAC address */
@@ -288,7 +286,7 @@ struct net_pkt_xmt {intlen;/* full length of data in the packet */intnum_frags;/* number of fragments in frags containing data */structphys_infofrags[MAX_PHYS_INFO];/* physical page information */-charethhdr[ETH_HEADER_SIZE];/* the ethernet header */+charethhdr[ETH_HLEN];/* the ethernet header */struct{/* these are needed for csum at uisnic end */u8valid;/* 1 = struct is valid - else ignore */
@@ -864,7 +864,7 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)/* copy ethernet header from first frag into ocmdrsp*-everythingelsewillbepassinfrags&DMA'ed*/-memcpy(cmdrsp->net.xmt.ethhdr,skb->data,ETH_HEADER_SIZE);+memcpy(cmdrsp->net.xmt.ethhdr,skb->data,ETH_HLEN);/* copy frags info - from skb->data we need to only provide access*beyondethheader*/
From: Jarod Wilson <hidden> Date: 2016-10-20 17:57:35
geneve:
- Merge __geneve_change_mtu back into geneve_change_mtu, set max_mtu
- This one isn't quite as straight-forward as others, could use some
closer inspection and testing
macvlan:
- set min/max_mtu
tun:
- set min/max_mtu, remove tun_net_change_mtu
vxlan:
- Merge __vxlan_change_mtu back into vxlan_change_mtu
- Set max_mtu to IP_MAX_MTU and retain dynamic MTU range checks in
change_mtu function
- This one is also not as straight-forward and could use closer inspection
and testing from vxlan folks
bridge:
- set max_mtu of IP_MAX_MTU and retain dynamic MTU range checks in
change_mtu function
openvswitch:
- set min/max_mtu, remove internal_dev_change_mtu
- note: max_mtu wasn't checked previously, it's been set to 65535, which
is the largest possible size supported
sch_teql:
- set min/max_mtu (note: max_mtu previously unchecked, used max of 65535)
macsec:
- min_mtu = 0, max_mtu = 65535
macvlan:
- min_mtu = 0, max_mtu = 65535
ntb_netdev:
- min_mtu = 0, max_mtu = 65535
veth:
- min_mtu = 68, max_mtu = 65535
8021q:
- min_mtu = 0, max_mtu = 65535
CC: netdev@vger.kernel.org
CC: Nicolas Dichtel <redacted>
CC: Hannes Frederic Sowa <redacted>
CC: Tom Herbert <redacted>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Alexander Duyck <redacted>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Jiri Benc <redacted>
CC: WANG Cong <redacted>
CC: Roopa Prabhu <redacted>
CC: Pravin B Shelar <redacted>
CC: Sabrina Dubroca <sd@queasysnail.net>
CC: Patrick McHardy <redacted>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: Pravin Shelar <redacted>
CC: Maxim Krasnyansky <redacted>
Signed-off-by: Jarod Wilson <redacted>
---
drivers/net/geneve.c | 48 +++++++++++----------------
drivers/net/macsec.c | 2 ++
drivers/net/macvlan.c | 8 ++++-
drivers/net/ntb_netdev.c | 3 ++
drivers/net/tun.c | 20 ++++-------
drivers/net/veth.c | 17 ++--------
drivers/net/vxlan.c | 64 +++++++++++++++++++-----------------
net/8021q/vlan_dev.c | 3 ++
net/bridge/br_device.c | 3 +-
net/openvswitch/vport-internal_dev.c | 10 ------
net/sched/sch_teql.c | 5 ++-
11 files changed, 81 insertions(+), 102 deletions(-)
@@ -1034,39 +1034,18 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)returngeneve_xmit_skb(skb,dev,info);}-staticint__geneve_change_mtu(structnet_device*dev,intnew_mtu,boolstrict)+staticintgeneve_change_mtu(structnet_device*dev,intnew_mtu){-structgeneve_dev*geneve=netdev_priv(dev);-/* The max_mtu calculation does not take account of GENEVE-*options,toavoidexcludingpotentiallyvalid-*configurations.+/* Only possible if called internally, ndo_change_mtu path's new_mtu+*isguaranteedtobebetweendev->min_mtuanddev->max_mtu.*/-intmax_mtu=IP_MAX_MTU-GENEVE_BASE_HLEN-dev->hard_header_len;--if(geneve->remote.sa.sa_family==AF_INET6)-max_mtu-=sizeof(structipv6hdr);-else-max_mtu-=sizeof(structiphdr);--if(new_mtu<68)-return-EINVAL;--if(new_mtu>max_mtu){-if(strict)-return-EINVAL;--new_mtu=max_mtu;-}+if(new_mtu>dev->max_mtu)+new_mtu=dev->max_mtu;dev->mtu=new_mtu;return0;}-staticintgeneve_change_mtu(structnet_device*dev,intnew_mtu)-{-return__geneve_change_mtu(dev,new_mtu,true);-}-staticintgeneve_fill_metadata_dst(structnet_device*dev,structsk_buff*skb){structip_tunnel_info*info=skb_tunnel_info(skb);
@@ -1170,6 +1149,14 @@ static void geneve_setup(struct net_device *dev)dev->hw_features|=NETIF_F_SG|NETIF_F_HW_CSUM|NETIF_F_RXCSUM;dev->hw_features|=NETIF_F_GSO_SOFTWARE;+/* MTU range: 68 - (something less than 65535) */+dev->min_mtu=ETH_MIN_MTU;+/* The max_mtu calculation does not take account of GENEVE+*options,toavoidexcludingpotentiallyvalid+*configurations.ThiswillbefurtherreducedbyIPvXhdrsize.+*/+dev->max_mtu=IP_MAX_MTU-GENEVE_BASE_HLEN-dev->hard_header_len;+netif_keep_dst(dev);dev->priv_flags&=~IFF_TX_SKB_SHARING;dev->priv_flags|=IFF_LIVE_ADDR_CHANGE|IFF_NO_QUEUE;
@@ -1285,10 +1272,13 @@ static int geneve_configure(struct net *net, struct net_device *dev,/* make enough headroom for basic scenario */encap_len=GENEVE_BASE_HLEN+ETH_HLEN;-if(remote->sa.sa_family==AF_INET)+if(remote->sa.sa_family==AF_INET){encap_len+=sizeof(structiphdr);-else+dev->max_mtu-=sizeof(structiphdr);+}else{encap_len+=sizeof(structipv6hdr);+dev->max_mtu-=sizeof(structipv6hdr);+}dev->needed_headroom=encap_len+ETH_HLEN;if(metadata){
@@ -1488,7 +1478,7 @@ struct net_device *geneve_dev_create_fb(struct net *net, const char *name,/* openvswitch users expect packet sizes to be unrestricted,*sosetthelargestMTUwecan.*/-err=__geneve_change_mtu(dev,IP_MAX_MTU,false);+err=geneve_change_mtu(dev,IP_MAX_MTU);if(err)gotoerr;
@@ -2367,43 +2367,31 @@ static void vxlan_set_multicast_list(struct net_device *dev){}-staticint__vxlan_change_mtu(structnet_device*dev,-structnet_device*lowerdev,-structvxlan_rdst*dst,intnew_mtu,boolstrict)+staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu){-intmax_mtu=IP_MAX_MTU;--if(lowerdev)-max_mtu=lowerdev->mtu;+structvxlan_dev*vxlan=netdev_priv(dev);+structvxlan_rdst*dst=&vxlan->default_dst;+structnet_device*lowerdev=__dev_get_by_index(vxlan->net,+dst->remote_ifindex);+booluse_ipv6=false;if(dst->remote_ip.sa.sa_family==AF_INET6)-max_mtu-=VXLAN6_HEADROOM;-else-max_mtu-=VXLAN_HEADROOM;--if(new_mtu<68)-return-EINVAL;+use_ipv6=true;-if(new_mtu>max_mtu){-if(strict)+/* This check is different than dev->max_mtu, because it looks at+*thelowerdev->mtu,ratherthanthestaticdev->max_mtu+*/+if(lowerdev){+intmax_mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+if(new_mtu>max_mtu)return-EINVAL;--new_mtu=max_mtu;}dev->mtu=new_mtu;return0;}-staticintvxlan_change_mtu(structnet_device*dev,intnew_mtu)-{-structvxlan_dev*vxlan=netdev_priv(dev);-structvxlan_rdst*dst=&vxlan->default_dst;-structnet_device*lowerdev=__dev_get_by_index(vxlan->net,-dst->remote_ifindex);-return__vxlan_change_mtu(dev,lowerdev,dst,new_mtu,true);-}-staticintvxlan_fill_metadata_dst(structnet_device*dev,structsk_buff*skb){structvxlan_dev*vxlan=netdev_priv(dev);
@@ -2795,6 +2783,10 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,vxlan_ether_setup(dev);}+/* MTU range: 68 - 65535 */+dev->min_mtu=ETH_MIN_MTU;+dev->max_mtu=ETH_MAX_MTU;+vxlan->net=src_net;dst->remote_vni=conf->vni;
@@ -2838,7 +2830,8 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,#endifif(!conf->mtu)-dev->mtu=lowerdev->mtu-(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);+dev->mtu=lowerdev->mtu-+(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);needed_headroom=lowerdev->hard_header_len;}elseif(vxlan_addr_multicast(&dst->remote_ip)){
@@ -2847,9 +2840,20 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,}if(conf->mtu){-err=__vxlan_change_mtu(dev,lowerdev,dst,conf->mtu,false);-if(err)-returnerr;+intmax_mtu=ETH_MAX_MTU;++if(lowerdev)+max_mtu=lowerdev->mtu;++max_mtu-=(use_ipv6?VXLAN6_HEADROOM:VXLAN_HEADROOM);++if(conf->mtu<dev->min_mtu||conf->mtu>dev->max_mtu)+return-EINVAL;++dev->mtu=conf->mtu;++if(conf->mtu>max_mtu)+dev->mtu=max_mtu;}if(use_ipv6||conf->flags&VXLAN_F_COLLECT_METADATA)
@@ -395,25 +395,6 @@ netdev_tx_t i2400m_hard_start_xmit(struct sk_buff *skb,static-inti2400m_change_mtu(structnet_device*net_dev,intnew_mtu)-{-intresult;-structi2400m*i2400m=net_dev_to_i2400m(net_dev);-structdevice*dev=i2400m_dev(i2400m);--if(new_mtu>=I2400M_MAX_MTU){-dev_err(dev,"Cannot change MTU to %d (max is %d)\n",-new_mtu,I2400M_MAX_MTU);-result=-EINVAL;-}else{-net_dev->mtu=new_mtu;-result=0;-}-returnresult;-}---staticvoidi2400m_tx_timeout(structnet_device*net_dev){/*
@@ -41,21 +41,6 @@ static int wil_stop(struct net_device *ndev)returnwil_down(wil);}-staticintwil_change_mtu(structnet_device*ndev,intnew_mtu)-{-structwil6210_priv*wil=ndev_to_wil(ndev);--if(new_mtu<68||new_mtu>mtu_max){-wil_err(wil,"invalid MTU %d\n",new_mtu);-return-EINVAL;-}--wil_dbg_misc(wil,"change MTU %d -> %d\n",ndev->mtu,new_mtu);-ndev->mtu=new_mtu;--return0;-}-staticintwil_do_ioctl(structnet_device*ndev,structifreq*ifr,intcmd){structwil6210_priv*wil=ndev_to_wil(ndev);
@@ -756,6 +743,11 @@ int wlan_setup(struct wlandevice *wlandev, struct device *physdev)wdev->wiphy=wiphy;wdev->iftype=NL80211_IFTYPE_STATION;netdev->ieee80211_ptr=wdev;+netdev->min_mtu=68;+/* 2312 is max 802.11 payload, 20 is overhead,+*(ether+llc+snap)andanother8forwep.+*/+netdev->max_mtu=(2312-20-8);netif_stop_queue(netdev);netif_carrier_off(netdev);
From: Jarod Wilson <hidden> Date: 2016-10-20 17:58:11
usbnet:
- Remove stale new_mtu <= 0 check in usbnet.c
- Set min_mtu = 0, max_mtu = 65535 (sub-drivers must set their own
max_mtu and/or min_mtu as needed)
r8152:
- Set appropriate max_mtu for different variants (1500 or 9194)
lan78xx:
- Set max_mtu = 9000
asix_driver:
- max_mtu = 16384 for ax88178 variant
ax88179:
- max_mtu = 4088
cdc_ncm:
- max_mtu from hardware
cdc-phonet:
- min_mtu = 6, max_mtu = 65541
sierra_net:
- max_mtu = 1500, call usbnet_change_mtu directly
- sierra_net_change_mtu checked for MTU > 1500, then called
usbnet_change_mtu, but if we set max_mtu to let the network core handle
the range check, then we can simply call usbnet_change_mtu directly
smsc75xx:
- max_mtu = 9000
CC: netdev@vger.kernel.org
CC: Woojung Huh <woojung.huh@microchip.com>
CC: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
CC: Hayes Wang <redacted>
CC: Oliver Neukum <oneukum@suse.com>
CC: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Jarod Wilson <redacted>
---
drivers/net/usb/asix_devices.c | 4 +---
drivers/net/usb/ax88179_178a.c | 4 +---
drivers/net/usb/cdc-phonet.c | 12 ++----------
drivers/net/usb/cdc_ncm.c | 5 +----
drivers/net/usb/lan78xx.c | 8 +++-----
drivers/net/usb/r8152.c | 15 ++++++++++++---
drivers/net/usb/sierra_net.c | 13 ++-----------
drivers/net/usb/smsc75xx.c | 4 +---
drivers/net/usb/usbnet.c | 4 ++--
9 files changed, 25 insertions(+), 44 deletions(-)
@@ -1980,11 +1980,6 @@ static int lan78xx_change_mtu(struct net_device *netdev, int new_mtu)intold_rx_urb_size=dev->rx_urb_size;intret;-if(new_mtu>MAX_SINGLE_PACKET_SIZE)-return-EINVAL;--if(new_mtu<=0)-return-EINVAL;/* no second zero-length packet read wanted after mtu-sized packets */if((ll_mtu%dev->maxpacket)==0)return-EDOM;
@@ -3388,6 +3383,9 @@ static int lan78xx_probe(struct usb_interface *intf,if(netdev->mtu>(dev->hard_mtu-netdev->hard_header_len))netdev->mtu=dev->hard_mtu-netdev->hard_header_len;+/* MTU range: 68 - 9000 */+netdev->max_mtu=MAX_SINGLE_PACKET_SIZE;+dev->ep_blkin=(intf->cur_altsetting)->endpoint+0;dev->ep_blkout=(intf->cur_altsetting)->endpoint+1;dev->ep_intr=(intf->cur_altsetting)->endpoint+2;
@@ -622,15 +621,6 @@ static const struct ethtool_ops sierra_net_ethtool_ops = {.nway_reset=usbnet_nway_reset,};-/* MTU can not be more than 1500 bytes, enforce it. */-staticintsierra_net_change_mtu(structnet_device*net,intnew_mtu)-{-if(new_mtu>SIERRA_NET_MAX_SUPPORTED_MTU)-return-EINVAL;--returnusbnet_change_mtu(net,new_mtu);-}-staticintsierra_net_get_fw_attr(structusbnet*dev,u16*datap){intresult=0;
@@ -720,6 +710,7 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)dev->net->hard_header_len+=SIERRA_NET_HIP_EXT_HDR_LEN;dev->hard_mtu=dev->net->mtu+dev->net->hard_header_len;+dev->net->max_mtu=SIERRA_NET_MAX_SUPPORTED_MTU;/* Set up the netdev */dev->net->flags|=IFF_NOARP;
@@ -925,9 +925,6 @@ static int smsc75xx_change_mtu(struct net_device *netdev, int new_mtu)structusbnet*dev=netdev_priv(netdev);intret;-if(new_mtu>MAX_SINGLE_PACKET_SIZE)-return-EINVAL;-ret=smsc75xx_set_rx_max_frame_length(dev,new_mtu+ETH_HLEN);if(ret<0){netdev_warn(dev->net,"Failed to set mac rx frame length\n");
@@ -384,8 +384,6 @@ int usbnet_change_mtu (struct net_device *net, int new_mtu)intold_hard_mtu=dev->hard_mtu;intold_rx_urb_size=dev->rx_urb_size;-if(new_mtu<=0)-return-EINVAL;// no second zero-length packet read wanted after mtu-sized packetsif((ll_mtu%dev->maxpacket)==0)return-EDOM;
-----Original Message-----
From: Jarod Wilson [mailto:jarod@redhat.com]
Sent: Thursday, October 20, 2016 1:55 PM
To: linux-kernel@vger.kernel.org
Cc: Jarod Wilson <redacted>; netdev@vger.kernel.org;
virtualization@lists.linux-foundation.org; KY Srinivasan
[off-list ref]; Haiyang Zhang [off-list ref]; Michael S.
Tsirkin [off-list ref]; Shrikrishna Khare [off-list ref]; VMware,
Inc. [off-list ref]; Wei Liu [off-list ref]; Paul
Durrant [off-list ref]; David Kershner
[off-list ref]
Subject: [PATCH net-next v2 6/9] net: use core MTU range checking in
virt drivers
hyperv_net:
- set min/max_mtu, per Haiyang, after rndis_filter_device_add
virtio_net:
- set min/max_mtu
- remove virtnet_change_mtu
vmxnet3:
- set min/max_mtu
xen-netback:
- min_mtu = 0, max_mtu = 65517
xen-netfront:
- min_mtu = 0, max_mtu = 65535
unisys/visor:
- clean up defines a little to not clash with network core or add
redundat definitions
CC: netdev@vger.kernel.org
CC: virtualization@lists.linux-foundation.org
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Shrikrishna Khare <redacted>
CC: "VMware, Inc." <redacted>
CC: Wei Liu <redacted>
CC: Paul Durrant <redacted>
CC: David Kershner <redacted>
Signed-off-by: Jarod Wilson <redacted>
---
The hv_netvsc changes look fine. Thanks.
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
From: Johannes Berg <johannes@sipsolutions.net> Date: 2016-10-20 18:22:41
On Thu, 2016-10-20 at 13:55 -0400, Jarod Wilson wrote:
- set max_mtu in wil6210 driver
- set max_mtu in atmel driver
- set min/max_mtu in cisco airo driver, remove airo_change_mtu
- set min/max_mtu in ipw2100/ipw2200 drivers, remove
libipw_change_mtu
- set min/max_mtu in p80211netdev, remove wlan_change_mtu
- set min/max_mtu in net/mac80211/iface.c and remove ieee80211_change_mtu
For the mac80211 part,
Acked-by: Johannes Berg <johannes@sipsolutions.net>
Dave, I'm assuming you'll pick this up, but if you prefer not to I can
also coordinate with Kalle to take this through our trees.
johannes
From: David Miller <davem@davemloft.net> Date: 2016-10-20 18:41:03
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 20 Oct 2016 20:22:35 +0200
On Thu, 2016-10-20 at 13:55 -0400, Jarod Wilson wrote:
quoted
- set max_mtu in wil6210 driver
- set max_mtu in atmel driver
- set min/max_mtu in cisco airo driver, remove airo_change_mtu
- set min/max_mtu in ipw2100/ipw2200 drivers, remove
libipw_change_mtu
- set min/max_mtu in p80211netdev, remove wlan_change_mtu
- set min/max_mtu in net/mac80211/iface.c and remove ieee80211_change_mtu
For the mac80211 part,
Acked-by: Johannes Berg <johannes@sipsolutions.net>
Dave, I'm assuming you'll pick this up, but if you prefer not to I can
also coordinate with Kalle to take this through our trees.
From: David Miller <davem@davemloft.net> Date: 2016-10-20 18:53:48
From: Jarod Wilson <redacted>
Date: Thu, 20 Oct 2016 13:55:15 -0400
This stack of patches should get absolutely everything in the kernel
converted from doing their own MTU range checking to the core MTU range
checking. This second spin includes alterations to hopefully fix all
concerns raised with the first, as well as including some additional
changes to drivers and infrastructure where I completely missed necessary
updates.
These have all been built through the 0-day build infrastructure via the
(rebasing) master branch at https://github.com/jarodwilson/linux-muck, which
at the time of the most recent compile across 147 configs, was based on
net-next at commit 7b1536ef0aa0.
Series applied, hopefully this gets most of the fallout.
Thanks Jarod.
@@ -872,19 +872,12 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)structnetvsc_device*nvdev=ndevctx->nvdev;structhv_device*hdev=ndevctx->device_ctx;structnetvsc_device_infodevice_info;-intlimit=ETH_DATA_LEN;u32num_chn;intret=0;if(ndevctx->start_remove||!nvdev||nvdev->destroy)return-ENODEV;-if(nvdev->nvsp_version>=NVSP_PROTOCOL_VERSION_2)-limit=NETVSC_MTU-ETH_HLEN;--if(mtu<NETVSC_MTU_MIN||mtu>limit)-return-EINVAL;-ret=netvsc_close(ndev);if(ret)gotoout;
@@ -1402,6 +1395,13 @@ static int netvsc_probe(struct hv_device *dev,netif_set_real_num_tx_queues(net,nvdev->num_chn);netif_set_real_num_rx_queues(net,nvdev->num_chn);+/* MTU range: 68 - 1500 or 65521 */+net->min_mtu=NETVSC_MTU_MIN;+if(nvdev->nvsp_version>=NVSP_PROTOCOL_VERSION_2)+net->max_mtu=NETVSC_MTU-ETH_HLEN;+else+net->max_mtu=ETH_DATA_LEN;+ret=register_netdev(net);if(ret!=0){pr_err("Unable to register netdev.\n");
@@ -113,12 +113,10 @@ enum net_types {};-#define ETH_HEADER_SIZE 14 /* size of ethernet header */-#define ETH_MIN_DATA_SIZE 46 /* minimum eth data size */-#define ETH_MIN_PACKET_SIZE (ETH_HEADER_SIZE + ETH_MIN_DATA_SIZE)+#define ETH_MIN_PACKET_SIZE (ETH_HLEN + ETH_MIN_DATA_SIZE)-#define ETH_MAX_MTU 16384 /* maximum data size */+#define VISOR_ETH_MAX_MTU 16384 /* maximum data size */#ifndef MAX_MACADDR_LEN#define MAX_MACADDR_LEN 6 /* number of bytes in MAC address */
@@ -288,7 +286,7 @@ struct net_pkt_xmt {intlen;/* full length of data in the packet */intnum_frags;/* number of fragments in frags containing data */structphys_infofrags[MAX_PHYS_INFO];/* physical page information */-charethhdr[ETH_HEADER_SIZE];/* the ethernet header */+charethhdr[ETH_HLEN];/* the ethernet header */struct{/* these are needed for csum at uisnic end */u8valid;/* 1 = struct is valid - else ignore */
@@ -864,7 +864,7 @@ visornic_xmit(struct sk_buff *skb, struct net_device *netdev)/* copy ethernet header from first frag into ocmdrsp*-everythingelsewillbepassinfrags&DMA'ed*/-memcpy(cmdrsp->net.xmt.ethhdr,skb->data,ETH_HEADER_SIZE);+memcpy(cmdrsp->net.xmt.ethhdr,skb->data,ETH_HLEN);/* copy frags info - from skb->data we need to only provide access*beyondethheader*/
From: Kershner, David A <hidden> Date: 2016-10-20 20:45:36
-----Original Message-----
From: Haiyang Zhang [mailto:haiyangz@microsoft.com]
Sent: Thursday, October 20, 2016 2:05 PM
To: Jarod Wilson <redacted>; linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; virtualization@lists.linux-foundation.org; KY
Srinivasan [off-list ref]; Michael S. Tsirkin [off-list ref];
Shrikrishna Khare [off-list ref]; VMware, Inc. <pv-
drivers@vmware.com>; Wei Liu [off-list ref]; Paul Durrant
[off-list ref]; Kershner, David A
[off-list ref]
Subject: RE: [PATCH net-next v2 6/9] net: use core MTU range checking in virt
drivers
quoted
-----Original Message-----
From: Jarod Wilson [mailto:jarod@redhat.com]
Sent: Thursday, October 20, 2016 1:55 PM
To: linux-kernel@vger.kernel.org
Cc: Jarod Wilson <redacted>; netdev@vger.kernel.org;
virtualization@lists.linux-foundation.org; KY Srinivasan
[off-list ref]; Haiyang Zhang [off-list ref];
Inc. [off-list ref]; Wei Liu [off-list ref]; Paul
Durrant [off-list ref]; David Kershner
[off-list ref]
Subject: [PATCH net-next v2 6/9] net: use core MTU range checking in
virt drivers
hyperv_net:
- set min/max_mtu, per Haiyang, after rndis_filter_device_add
virtio_net:
- set min/max_mtu
- remove virtnet_change_mtu
vmxnet3:
- set min/max_mtu
xen-netback:
- min_mtu = 0, max_mtu = 65517
xen-netfront:
- min_mtu = 0, max_mtu = 65535
unisys/visor:
- clean up defines a little to not clash with network core or add
redundat definitions
CC: netdev@vger.kernel.org
CC: virtualization@lists.linux-foundation.org
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Shrikrishna Khare <redacted>
CC: "VMware, Inc." <redacted>
CC: Wei Liu <redacted>
CC: Paul Durrant <redacted>
CC: David Kershner <redacted>
Signed-off-by: Jarod Wilson <redacted>
---
The hv_netvsc changes look fine. Thanks.
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
The visornic changes look good.
Reviewed-by: David Kershner <redacted>
Bah. Yeah. Should have just used them directly. I didn't add ETH_MAX_MTU
until after doing the virtio_net changes, so I missed that.
quoted
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev) dev->vlan_features = dev->features;+ /* MTU range: 68 - 65535 */+ dev->min_mtu = MIN_MTU;+ dev->max_mtu = MAX_MTU;+ /* Configuration may specify what MAC to use. Otherwise random. */ if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) virtio_cread_bytes(vdev,
@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev) mtu = virtio_cread16(vdev, offsetof(struct virtio_net_config, mtu));- if (virtnet_change_mtu(dev, mtu))+ if (mtu < dev->min_mtu || mtu > dev->max_mtu)
In fact the > max_mtu branch does not make sense since a 16 bit
value can't exceed MAX_MTU.
Hm. mtu is declared as an int, not sure if there's any sort of type
promotion to be worried about (not an area I know much/anything about).
Certainly something that could be looked into as a minor optimization,
though it's only in a probe path and shouldn't hurt anything, so ... meh?
--
Jarod Wilson
jarod@redhat.com
From: Sven Eckelmann <sven@narfation.org> Date: 2016-10-22 07:25:56
On Donnerstag, 20. Oktober 2016 13:55:22 CEST Jarod Wilson wrote:
[...]
batman-adv:
- set max_mtu
- remove batadv_interface_change_mtu
- initialization is a little async, not 100% certain that max_mtu is set
in the optimal place, don't have hardware to test with
batman-adv is creating a virtual interface - so there are no
hardware requirements (ok, ethernet compatible hardware - even
when only virtual/emulated).
[...]
This looks bogus to me. You are now setting max_mtu during initialization of
the virtual interface. But at this time no slave interfaces were added to the
master batman-adv interface. So the batadv_hardif_min_mtu will not return the
correct value here. Especially if you don't have fragmentation enabled.
So this change looks like a bug to me
Kind regards,
Sven
I need to check more closely, but I think the RFC 2734 encapsulation spec
and our implementation do not impose a particular upper limit. Though I
guess it's bad to let userland set arbitrarily large values here.
In which case, that would suggest using IP_MAX_MTU (65535) here.
Probably. I (or somebody) need to check the spec and the code once more.
[...]
But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
However, there's an explicit calculation for 'max_mtu' right there that I
glazed right over. It would seem perhaps *that* should be used for
net->max_mtu here, no?
No. This 'max_mtu' here is not the absolute maximum. It is only an
initial MTU which has the property that link fragmentation is not
going to happen (if all other peers will at least as capable as this
node).
--
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
However, there's an explicit calculation for 'max_mtu' right there that I
glazed right over. It would seem perhaps *that* should be used for
net->max_mtu here, no?
No. This 'max_mtu' here is not the absolute maximum. It is only an
initial MTU which has the property that link fragmentation is not
going to happen (if all other peers will at least as capable as this
node).
Besides, card->max_receive is about what the card can receive (at the IEEE
1394 link layer), not about what the card can send.
--
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/
@@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit, * Use the RFC 2734 default 1500 octets or the maximum payload * as initial MTU */- max_mtu = (1 << (card->max_receive + 1))- - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;- net->mtu = min(1500U, max_mtu);+ net->max_mtu = (1 << (card->max_receive + 1))+ - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;+ net->mtu = min(1500U, net->max_mtu);+ net->min_mtu = ETH_MIN_MTU; /* Set our hardware address while we're at it */ ha = (union fwnet_hwaddr *)net->dev_addr;
Please preserve the current behavior, i.e. do not enforce any particular
upper bound. (Especially none based on the local link layer controller's
max_receive parameter.)
BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
that net->mtu should be initialized to 1500, not less. But such a change
should be done in a separate patch.
--
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/
@@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit, struct net_device *net; bool allocated_netdev = false; struct fwnet_device *dev;- unsigned max_mtu; int ret; union fwnet_hwaddr *ha;
@@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit, * Use the RFC 2734 default 1500 octets or the maximum payload * as initial MTU */- max_mtu = (1 << (card->max_receive + 1))- - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;- net->mtu = min(1500U, max_mtu);+ net->max_mtu = (1 << (card->max_receive + 1))+ - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;+ net->mtu = min(1500U, net->max_mtu);+ net->min_mtu = ETH_MIN_MTU; /* Set our hardware address while we're at it */ ha = (union fwnet_hwaddr *)net->dev_addr;
Please preserve the current behavior, i.e. do not enforce any particular
upper bound. (Especially none based on the local link layer controller's
max_receive parameter.)
BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
that net->mtu should be initialized to 1500, not less. But such a change
should be done in a separate patch.
@@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit, struct net_device *net; bool allocated_netdev = false; struct fwnet_device *dev;- unsigned max_mtu; int ret; union fwnet_hwaddr *ha;
@@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit, * Use the RFC 2734 default 1500 octets or the maximum payload * as initial MTU */- max_mtu = (1 << (card->max_receive + 1))- - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;- net->mtu = min(1500U, max_mtu);+ net->max_mtu = (1 << (card->max_receive + 1))+ - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;+ net->mtu = min(1500U, net->max_mtu);+ net->min_mtu = ETH_MIN_MTU; /* Set our hardware address while we're at it */ ha = (union fwnet_hwaddr *)net->dev_addr;
Please preserve the current behavior, i.e. do not enforce any particular
upper bound. (Especially none based on the local link layer controller's
max_receive parameter.)
BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
that net->mtu should be initialized to 1500, not less. But such a change
should be done in a separate patch.
Okay, since it's already merged in net-next, I can do a follow-up patch
here to set max_mtu to ETH_MAX_MTU (65535), which is the largest possible
size the kernel can handle, so far as I can tell. But as long as I'm going
to be in here, if we just want to use an initial mtu of 1500, I could
clean that up at the same time, and entirely remove the max_mtu
calculation stuff, if that's what you think is more correct here.
--
Jarod Wilson
jarod@redhat.com
From: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: 2016-10-23 14:29:27
Commit b3e3893e1253 ("net: use core MTU range checking in misc drivers")
mistakenly introduced an upper limit for firewire-net's MTU based on the
local link layer controller's reception capability. Revert this. Neither
RFC 2734 nor our implementation impose any particular upper limit.
Actually, to be on the safe side and to make the code explicit, set
ETH_MAX_MTU = 65535 as upper limit now.
(I replaced sizeof(struct rfc2734_header) by the equivalent
RFC2374_FRAG_HDR_SIZE in order to avoid distracting long/int conversions.)
Fixes: b3e3893e1253('net: use core MTU range checking in misc drivers')
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
@@ -1467,10 +1467,11 @@ static int fwnet_probe(struct fw_unit *unit,*UsetheRFC2734default1500octetsorthemaximumpayload*asinitialMTU*/-net->max_mtu=(1<<(card->max_receive+1))--sizeof(structrfc2734_header)-IEEE1394_GASP_HDR_SIZE;-net->mtu=min(1500U,net->max_mtu);+net->mtu=min(1500U,+(1U<<(card->max_receive+1))+-RFC2374_FRAG_HDR_SIZE-IEEE1394_GASP_HDR_SIZE);net->min_mtu=ETH_MIN_MTU;+net->max_mtu=ETH_MAX_MTU;/* Set our hardware address while we're at it */ha=(unionfwnet_hwaddr*)net->dev_addr;
--
Stefan Richter
-======----- =-=- =-===
http://arcgraph.de/sr/
From: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: 2016-10-23 14:31:14
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
@@ -1463,13 +1463,7 @@ static int fwnet_probe(struct fw_unit *unit,gotoout;dev->local_fifo=dev->handler.offset;-/*-*UsetheRFC2734default1500octetsorthemaximumpayload-*asinitialMTU-*/-net->mtu=min(1500U,-(1U<<(card->max_receive+1))--RFC2374_FRAG_HDR_SIZE-IEEE1394_GASP_HDR_SIZE);+net->mtu=1500U;net->min_mtu=ETH_MIN_MTU;net->max_mtu=ETH_MAX_MTU;
--
Stefan Richter
-======----- =-=- =-===
http://arcgraph.de/sr/
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
From: Jarod Wilson <hidden> Date: 2016-10-24 01:50:20
On Sun, Oct 23, 2016 at 04:30:56PM +0200, Stefan Richter wrote:
quoted hunk
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
From: Jarod Wilson <hidden> Date: 2016-10-24 01:50:47
On Sun, Oct 23, 2016 at 04:29:03PM +0200, Stefan Richter wrote:
Commit b3e3893e1253 ("net: use core MTU range checking in misc drivers")
mistakenly introduced an upper limit for firewire-net's MTU based on the
local link layer controller's reception capability. Revert this. Neither
RFC 2734 nor our implementation impose any particular upper limit.
Actually, to be on the safe side and to make the code explicit, set
ETH_MAX_MTU = 65535 as upper limit now.
(I replaced sizeof(struct rfc2734_header) by the equivalent
RFC2374_FRAG_HDR_SIZE in order to avoid distracting long/int conversions.)
Fixes: b3e3893e1253('net: use core MTU range checking in misc drivers')
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Acked-by: Jarod Wilson <redacted>
--
Jarod Wilson
jarod@redhat.com
From: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: 2016-10-24 12:26:39
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
v2: use ETH_DATA_LEN, add comment
drivers/firewire/net.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
@@ -1463,13 +1463,8 @@ static int fwnet_probe(struct fw_unit *unit,gotoout;dev->local_fifo=dev->handler.offset;-/*-*UsetheRFC2734default1500octetsorthemaximumpayload-*asinitialMTU-*/-net->mtu=min(1500U,-(1U<<(card->max_receive+1))--RFC2374_FRAG_HDR_SIZE-IEEE1394_GASP_HDR_SIZE);+/* MTU range: 68 - 65535, RFC 2734 default: 1500 */+net->mtu=ETH_DATA_LEN;net->min_mtu=ETH_MIN_MTU;net->max_mtu=ETH_MAX_MTU;
--
2.7.3
--
Stefan Richter
-======----- =-=- ==---
http://arcgraph.de/sr/
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
From: Jarod Wilson <hidden> Date: 2016-10-25 03:05:26
On Mon, Oct 24, 2016 at 02:26:13PM +0200, Stefan Richter wrote:
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
v2: use ETH_DATA_LEN, add comment
Acked-by: Jarod Wilson <redacted>
--
Jarod Wilson
jarod@redhat.com
From: David Miller <davem@davemloft.net> Date: 2016-10-26 21:29:56
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sun, 23 Oct 2016 16:30:56 +0200
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Applied.
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
From: David Miller <davem@davemloft.net> Date: 2016-10-26 21:29:58
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sun, 23 Oct 2016 16:29:03 +0200
Commit b3e3893e1253 ("net: use core MTU range checking in misc drivers")
mistakenly introduced an upper limit for firewire-net's MTU based on the
local link layer controller's reception capability. Revert this. Neither
RFC 2734 nor our implementation impose any particular upper limit.
Actually, to be on the safe side and to make the code explicit, set
ETH_MAX_MTU = 65535 as upper limit now.
(I replaced sizeof(struct rfc2734_header) by the equivalent
RFC2374_FRAG_HDR_SIZE in order to avoid distracting long/int conversions.)
Fixes: b3e3893e1253('net: use core MTU range checking in misc drivers')
CC: netdev@vger.kernel.org
CC: linux1394-devel@lists.sourceforge.net
CC: Jarod Wilson <redacted>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Applied.
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
From: Stefan Richter <stefanr@s5r6.in-berlin.de> Date: 2016-10-29 20:17:20
The maximum unicast datagram size /without/ link fragmentation is
4096 - 4 = 4092 (max IEEE 1394 async payload size at >= S800 bus speed,
minus unfragmented encapssulation header). Max broadcast datagram size
without fragmentation is 8 bytes less than that (due to GASP header).
The maximum datagram size /with/ link fragmentation is 0xfff = 4095
for unicast and broadcast. This is because the RFC 2734 fragment
encapsulation header field for datagram size is only 12 bits wide.
Fixes: 5d48f00d836a('firewire: net: fix maximum possible MTU')
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1465,7 +1465,7 @@ static int fwnet_probe(struct fw_unit *unit,net->mtu=1500U;net->min_mtu=ETH_MIN_MTU;-net->max_mtu=ETH_MAX_MTU;+net->max_mtu=0xfff;/* Set our hardware address while we're at it */ha=(unionfwnet_hwaddr*)net->dev_addr;
--
Stefan Richter
-======----- =-=- ===-=
http://arcgraph.de/sr/
From: David Miller <davem@davemloft.net> Date: 2016-10-30 03:01:08
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat, 29 Oct 2016 22:16:58 +0200
The maximum unicast datagram size /without/ link fragmentation is
4096 - 4 = 4092 (max IEEE 1394 async payload size at >= S800 bus speed,
minus unfragmented encapssulation header). Max broadcast datagram size
without fragmentation is 8 bytes less than that (due to GASP header).
The maximum datagram size /with/ link fragmentation is 0xfff = 4095
for unicast and broadcast. This is because the RFC 2734 fragment
encapsulation header field for datagram size is only 12 bits wide.
Fixes: 5d48f00d836a('firewire: net: fix maximum possible MTU')
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>