svn commit: r316699 - head/sys/net

[Rodney W. Grimes]

[ Charset UTF-8 unsupported, converting... ]
> On 11.04.2017 19:51, Rodney W. Grimes wrote:
> >> Modified: head/sys/net/if_gre.c
> >> ==============================================================================
> >> --- head/sys/net/if_gre.c	Tue Apr 11 08:29:12 2017	(r316698)
> >> +++ head/sys/net/if_gre.c	Tue Apr 11 08:56:18 2017	(r316699)
> >> @@ -88,7 +88,7 @@ __FBSDID("$FreeBSD$");
> >>  #include <machine/in_cksum.h>
> >>  #include <security/mac/mac_framework.h>
> >>  
> >> -#define	GREMTU			1500
> >> +#define	GREMTU			1476
> > 
> > I would of thought that this was
> > #define	GREMTU	ETHERMTU
> > and now should probably be:
> > #define	GREMTU	ETHERMTU - gre_hlen; 
> > Or what ever the approprite sizeof(foo) is;
> 
> I just returned back constants that were here :)

Can we try to improve on this, or maybe just ignore it?

> The man page says:
>  "The MTU of gre interfaces is set to 1476 by default, to match the
>   value used by Cisco routers."

That might need some clean up too?

> > Isn't this arguably wrong in the face of JumboFrames?
> >
> 
> I don't see why it may be wrong for jumbo frames. The default MTU value
> used by any interfaces is ETHERMTU, if you know that your NIC supports
> jumbo frames, you will configure specific MTU value.
> If you want 9k MTU on gre(4), you should configure it.

I may some day want to #define ETHERMTU 9000 and just have all the right
magic happen, wouldnt that be nice?

> >>  static const char grename[] = "gre";
> >>  static MALLOC_DEFINE(M_GRE, grename, "Generic Routing Encapsulation");
> >>  static VNET_DEFINE(struct mtx, gre_mtx);
> >> @@ -173,7 +173,7 @@ gre_clone_create(struct if_clone *ifc, i
> >>  	GRE2IFP(sc)->if_softc = sc;
> >>  	if_initname(GRE2IFP(sc), grename, unit);
> >>  
> >> -	GRE2IFP(sc)->if_mtu = sc->gre_mtu = GREMTU;
> >> +	GRE2IFP(sc)->if_mtu = GREMTU;
> >>  	GRE2IFP(sc)->if_flags = IFF_POINTOPOINT|IFF_MULTICAST;
> >>  	GRE2IFP(sc)->if_output = gre_output;
> >>  	GRE2IFP(sc)->if_ioctl = gre_ioctl;
> >> @@ -231,7 +231,8 @@ gre_ioctl(struct ifnet *ifp, u_long cmd,
> >>  		 /* XXX: */
> >>  		if (ifr->ifr_mtu < 576)
> > 
> > This is another magic constant that should be in a #define.
> 
> It was here too. I think it is something from the past and can be removed.
> 
> From RFC791: "It is recommended that hosts only send datagrams larger
> than 576 octets".

No, I doubt we want to remove that, it is also part of ipv6 documentation,
and iirc part of some BCP's.  You really dont want to be running on the
internet with MTU < 576.  It use to be 296 in the days when we had SLIP.

> 
> >>  			return (EINVAL);
> >> -		break;
> >> +		ifp->if_mtu = ifr->ifr_mtu;
> >> +		return (0);
> >>  	case SIOCSIFADDR:
> >>  		ifp->if_flags |= IFF_UP;
> >>  	case SIOCSIFFLAGS:
> >> @@ -255,12 +256,6 @@ gre_ioctl(struct ifnet *ifp, u_long cmd,
> >>  	}
> >>  	error = 0;
> >>  	switch (cmd) {
> >> -	case SIOCSIFMTU:
> >> -		GRE_WLOCK(sc);
> >> -		sc->gre_mtu = ifr->ifr_mtu;
> >> -		gre_updatehdr(sc);
> >> -		GRE_WUNLOCK(sc);
> >> -		goto end;
> >>  	case SIOCSIFPHYADDR:
> >>  #ifdef INET6
> >>  	case SIOCSIFPHYADDR_IN6:
> >> @@ -549,7 +544,6 @@ gre_updatehdr(struct gre_softc *sc)
> >>  	} else
> >>  		sc->gre_oseq = 0;
> >>  	gh->gre_flags = htons(flags);
> >> -	GRE2IFP(sc)->if_mtu = sc->gre_mtu - sc->gre_hlen;
> >>  }
> >>  
> >>  static void
> >>
> >> Modified: head/sys/net/if_gre.h
> >> ==============================================================================
> >> --- head/sys/net/if_gre.h	Tue Apr 11 08:29:12 2017	(r316698)
> >> +++ head/sys/net/if_gre.h	Tue Apr 11 08:56:18 2017	(r316699)
> >> @@ -69,7 +69,6 @@ struct gre_softc {
> >>  	uint32_t		gre_oseq;
> >>  	uint32_t		gre_key;
> >>  	uint32_t		gre_options;
> >> -	uint32_t		gre_mtu;
> >>  	u_int			gre_fibnum;
> >>  	u_int			gre_hlen;	/* header size */
> >>  	union {
> >>
> >> Modified: head/sys/net/if_me.c
> >> ==============================================================================
> >> --- head/sys/net/if_me.c	Tue Apr 11 08:29:12 2017	(r316698)
> >> +++ head/sys/net/if_me.c	Tue Apr 11 08:56:18 2017	(r316699)
> >> @@ -66,7 +66,7 @@ __FBSDID("$FreeBSD$");
> >>  #include <machine/in_cksum.h>
> >>  #include <security/mac/mac_framework.h>
> >>  
> >> -#define	MEMTU			1500
> >> +#define	MEMTU			(1500 - sizeof(struct mobhdr))
> > 
> > 1500 -> ETHERMTU?
> 
> Feel free to fix this. Many pseudo interfaces has similar MTU limitations.
> 
> -- 
> WBR, Andrey V. Elsukov
> 

-- 
Rod Grimes                                                 rgrimes at freebsd.org