From owner-svn-src-all@freebsd.org Wed Jan 22 20:59:24 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 84D441FD3E4; Wed, 22 Jan 2020 20:59:24 +0000 (UTC) (envelope-from melifaro@ipfw.ru) Received: from forward501j.mail.yandex.net (forward501j.mail.yandex.net [5.45.198.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 482ySr1XJMz4Wm5; Wed, 22 Jan 2020 20:59:23 +0000 (UTC) (envelope-from melifaro@ipfw.ru) Received: from mxback4q.mail.yandex.net (mxback4q.mail.yandex.net [IPv6:2a02:6b8:c0e:6d:0:640:ed15:d2bd]) by forward501j.mail.yandex.net (Yandex) with ESMTP id EC87233800AE; Wed, 22 Jan 2020 23:59:19 +0300 (MSK) Received: from localhost (localhost [::1]) by mxback4q.mail.yandex.net (mxback/Yandex) with ESMTP id KXNU8lNT1a-xJP0Pati; Wed, 22 Jan 2020 23:59:19 +0300 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfw.ru; s=mail; t=1579726759; bh=mV80VBFykBefnCBE0eUKvHUo55p8yfmwCZYpC8eGK4I=; h=Message-Id:Subject:In-Reply-To:Date:References:To:From; b=wEawV13YsXQuYUaFOwH0q2ykxf0GEktGV0EYTybx+ZJy3iePz4VYPHcD6HJsEt2fz 5zm7l9chZ+OpKU3QskOD5a+3L74t7mjutT3leeS/PadjQftkv4xbxEJo5vtBMIcT5c FGrEaGYzr4NVPGndSNvVq/3Z6cU5w3qkzpIigVzI= Received: by vla5-4452c58d5c14.qloud-c.yandex.net with HTTP; Wed, 22 Jan 2020 23:59:19 +0300 From: Alexander V. Chernikov Envelope-From: melifaro@ipfw.ru To: Alexander Motin , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" In-Reply-To: <202001222036.00MKakdv056453@repo.freebsd.org> References: <202001222036.00MKakdv056453@repo.freebsd.org> Subject: Re: svn commit: r356993 - head/sys/net MIME-Version: 1.0 X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Wed, 22 Jan 2020 20:59:19 +0000 Message-Id: <186041579726759@vla5-4452c58d5c14.qloud-c.yandex.net> Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=utf-8 X-Rspamd-Queue-Id: 482ySr1XJMz4Wm5 X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-6.00 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.997,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jan 2020 20:59:24 -0000 22.01.2020, 20:36, "Alexander Motin" : > Author: mav > Date: Wed Jan 22 20:36:45 2020 > New Revision: 356993 > URL: https://svnweb.freebsd.org/changeset/base/356993 > > Log: >   Update route MTUs for bridge, lagg and vlan interfaces. > >   Those interfaces may implicitly change their MTU on addition of parent >   interface in addition to normal SIOCSIFMTU ioctl path, where the route >   MTUs are updated normally. > >   MFC after: 2 weeks >   Sponsored by: iXsystems, Inc. > > Modified: >   head/sys/net/if_bridge.c >   head/sys/net/if_lagg.c >   head/sys/net/if_vlan.c I'd suggest not duplicating business logic on providing notifications to multiple subsystems in each driver. This adds unnecessary complexity/layer violations. What do you think of creating something like if_notify_mtu_change(), embed these 2 calls there and use it everywhere? > > Modified: head/sys/net/if_bridge.c > ============================================================================== > --- head/sys/net/if_bridge.c Wed Jan 22 20:31:01 2020 (r356992) > +++ head/sys/net/if_bridge.c Wed Jan 22 20:36:45 2020 (r356993) > @@ -135,7 +135,15 @@ __FBSDID("$FreeBSD$"); > >  #include > > +#ifdef INET6 >  /* > + * XXX: declare here to avoid to include many inet6 related files.. > + * should be more generalized? > + */ > +extern void nd6_setmtu(struct ifnet *); > +#endif > + > +/* >   * Size of the route hash table. Must be a power of two. >   */ >  #ifndef BRIDGE_RTHASH_SIZE > @@ -772,7 +780,7 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t da >          } args; >          struct ifdrv *ifd = (struct ifdrv *) data; >          const struct bridge_control *bc; > - int error = 0; > + int error = 0, oldmtu; > >          switch (cmd) { > > @@ -818,11 +826,23 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t da >                                  break; >                  } > > + oldmtu = ifp->if_mtu; >                  BRIDGE_LOCK(sc); >                  error = (*bc->bc_func)(sc, &args); >                  BRIDGE_UNLOCK(sc); >                  if (error) >                          break; > + > + /* > + * Bridge MTU may change during addition of the first port. > + * If it did, do network layer specific procedure. > + */ > + if (ifp->if_mtu != oldmtu) { > +#ifdef INET6 > + nd6_setmtu(ifp); > +#endif > + rt_updatemtu(ifp); > + } > >                  if (bc->bc_flags & BC_F_COPYOUT) >                          error = copyout(&args, ifd->ifd_data, ifd->ifd_len); > > Modified: head/sys/net/if_lagg.c > ============================================================================== > --- head/sys/net/if_lagg.c Wed Jan 22 20:31:01 2020 (r356992) > +++ head/sys/net/if_lagg.c Wed Jan 22 20:36:45 2020 (r356993) > @@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$"); >  #include >  #include >  #include > +#include >  #include > >  #if defined(INET) || defined(INET6) > @@ -74,6 +75,14 @@ __FBSDID("$FreeBSD$"); >  #include >  #include > > +#ifdef INET6 > +/* > + * XXX: declare here to avoid to include many inet6 related files.. > + * should be more generalized? > + */ > +extern void nd6_setmtu(struct ifnet *); > +#endif > + >  #define LAGG_RLOCK() struct epoch_tracker lagg_et; epoch_enter_preempt(net_epoch_preempt, &lagg_et) >  #define LAGG_RUNLOCK() epoch_exit_preempt(net_epoch_preempt, &lagg_et) >  #define LAGG_RLOCK_ASSERT() NET_EPOCH_ASSERT() > @@ -1178,7 +1187,7 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data >          struct ifnet *tpif; >          struct thread *td = curthread; >          char *buf, *outbuf; > - int count, buflen, len, error = 0; > + int count, buflen, len, error = 0, oldmtu; > >          bzero(&rpbuf, sizeof(rpbuf)); > > @@ -1453,10 +1462,23 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data >                                      tpif->if_xname); >                  } >  #endif > + oldmtu = ifp->if_mtu; >                  LAGG_XLOCK(sc); >                  error = lagg_port_create(sc, tpif); >                  LAGG_XUNLOCK(sc); >                  if_rele(tpif); > + > + /* > + * LAGG MTU may change during addition of the first port. > + * If it did, do network layer specific procedure. > + */ > + if (ifp->if_mtu != oldmtu) { > +#ifdef INET6 > + nd6_setmtu(ifp); > +#endif > + rt_updatemtu(ifp); > + } > + >                  VLAN_CAPABILITIES(ifp); >                  break; >          case SIOCSLAGGDELPORT: > > Modified: head/sys/net/if_vlan.c > ============================================================================== > --- head/sys/net/if_vlan.c Wed Jan 22 20:31:01 2020 (r356992) > +++ head/sys/net/if_vlan.c Wed Jan 22 20:36:45 2020 (r356993) > @@ -46,6 +46,7 @@ >  __FBSDID("$FreeBSD$"); > >  #include "opt_inet.h" > +#include "opt_inet6.h" >  #include "opt_kern_tls.h" >  #include "opt_vlan.h" >  #include "opt_ratelimit.h" > @@ -75,6 +76,7 @@ __FBSDID("$FreeBSD$"); >  #include >  #include >  #include > +#include >  #include > >  #ifdef INET > @@ -82,6 +84,14 @@ __FBSDID("$FreeBSD$"); >  #include >  #endif > > +#ifdef INET6 > +/* > + * XXX: declare here to avoid to include many inet6 related files.. > + * should be more generalized? > + */ > +extern void nd6_setmtu(struct ifnet *); > +#endif > + >  #define VLAN_DEF_HWIDTH 4 >  #define VLAN_IFFLAGS (IFF_BROADCAST | IFF_MULTICAST) > > @@ -1807,7 +1817,7 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data >          struct ifvlan *ifv; >          struct ifvlantrunk *trunk; >          struct vlanreq vlr; > - int error = 0; > + int error = 0, oldmtu; > >          ifr = (struct ifreq *)data; >          ifa = (struct ifaddr *) data; > @@ -1901,8 +1911,20 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data >                          error = ENOENT; >                          break; >                  } > + oldmtu = ifp->if_mtu; >                  error = vlan_config(ifv, p, vlr.vlr_tag); >                  if_rele(p); > + > + /* > + * VLAN MTU may change during addition of the vlandev. > + * If it did, do network layer specific procedure. > + */ > + if (ifp->if_mtu != oldmtu) { > +#ifdef INET6 > + nd6_setmtu(ifp); > +#endif > + rt_updatemtu(ifp); > + } >                  break; > >          case SIOCGETVLAN: