On Tue, Apr 18, 2023 at 06:49:39PM +0300, Vitaliy Makkoveev wrote:
> rtable_setsource(... , NULL) actually doesn't destroy object pointed by
> `ar_source', so for this call netlock is not required. However this
> optimisation doesn't produce any visible effect, so no objections to
> call rt_setsource() with netlock held.
>
> ok?
OK bluhm@
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 if_umb.c
> --- sys/dev/usb/if_umb.c 31 Mar 2023 23:53:49 -0000 1.50
> +++ sys/dev/usb/if_umb.c 18 Apr 2023 15:45:04 -0000
> @@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc
> default_sin.sin_len = sizeof (default_sin);
>
> memset(&info, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sintosa(&ifra.ifra_addr),
> ifp->if_rdomain);
> @@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc
> info.rti_info[RTAX_NETMASK] = sintosa(&default_sin);
> info.rti_info[RTAX_GATEWAY] = sintosa(&ifra.ifra_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> @@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *s
> default_sin6.sin6_len = sizeof (default_sin6);
>
> memset(&info, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> ifp->if_rdomain);
> @@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *s
> info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.688
> diff -u -p -r1.688 if.c
> --- sys/net/if.c 8 Apr 2023 13:49:38 -0000 1.688
> +++ sys/net/if.c 18 Apr 2023 15:45:04 -0000
> @@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> struct ifaddr *ifa;
> u_int rdomain;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rtableid);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1420,12 +1421,10 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> continue;
>
> if (equal(addr, ifa->ifa_addr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
>
> @@ -1438,8 +1437,9 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
> struct ifnet *ifp;
> struct ifaddr *ifa;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rdomain);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1449,13 +1449,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
> addr->sa_family || ifa->ifa_dstaddr == NULL)
> continue;
> if (equal(addr, ifa->ifa_dstaddr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
>
> @@ -3167,12 +3165,14 @@ ifsettso(struct ifnet *ifp, int on)
> void
> ifa_add(struct ifnet *ifp, struct ifaddr *ifa)
> {
> + NET_ASSERT_LOCKED_EXCLUSIVE();
> TAILQ_INSERT_TAIL(&ifp->if_addrlist, ifa, ifa_list);
> }
>
> void
> ifa_del(struct ifnet *ifp, struct ifaddr *ifa)
> {
> + NET_ASSERT_LOCKED_EXCLUSIVE();
> TAILQ_REMOVE(&ifp->if_addrlist, ifa, ifa_list);
> }
>
> Index: sys/net/if_var.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.123
> diff -u -p -r1.123 if_var.h
> --- sys/net/if_var.h 5 Apr 2023 19:35:23 -0000 1.123
> +++ sys/net/if_var.h 18 Apr 2023 15:45:04 -0000
> @@ -240,7 +240,8 @@ struct ifaddr {
> #define ifa_broadaddr ifa_dstaddr /* broadcast address interface
> */
> struct sockaddr *ifa_netmask; /* used to determine subnet */
> struct ifnet *ifa_ifp; /* back-pointer to interface */
> - TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
> + TAILQ_ENTRY(ifaddr) ifa_list; /* [N] list of addresses for
> + interface */
> u_int ifa_flags; /* interface flags, see below */
> struct refcnt ifa_refcnt; /* number of `rt_ifa` references */
> int ifa_metric; /* cost of going out this interface */
> Index: sys/net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.362
> diff -u -p -r1.362 rtsock.c
> --- sys/net/rtsock.c 18 Apr 2023 09:56:54 -0000 1.362
> +++ sys/net/rtsock.c 18 Apr 2023 15:45:04 -0000
> @@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct sock
> error = EINVAL;
> goto fail;
> }
> - if ((error =
> - rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
> + NET_LOCK();
> + error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
> + NET_UNLOCK();
> + if (error)
> goto fail;
> } else {
> error = rtm_output(rtm, &rt, &info, prio, tableid);
> @@ -2350,7 +2352,6 @@ int
> rt_setsource(unsigned int rtableid, struct sockaddr *src)
> {
> struct ifaddr *ifa;
> - int error;
> /*
> * If source address is 0.0.0.0 or ::
> * use automatic source selection
> @@ -2374,20 +2375,14 @@ rt_setsource(unsigned int rtableid, stru
> return (EAFNOSUPPORT);
> }
>
> - KERNEL_LOCK();
> /*
> * Check if source address is assigned to an interface in the
> * same rdomain
> */
> - if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL) {
> - KERNEL_UNLOCK();
> + if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL)
> return (EINVAL);
> - }
>
> - error = rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
> - KERNEL_UNLOCK();
> -
> - return (error);
> + return rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
> }
>
> /*
