Unneeded splnet()/splx() in carp(4)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Unneeded splnet()/splx() in carp(4)

Martin Pieuchot
carp(4), as a pseudo-interface, is always executed in the 'softnet'
thread.  Using splnet()/splx() might have been relevant when link-state
handlers where directly executed from hardware interrupt handlers.  But
nowadays everything is run under the NET_LOCK() in a thread context, so
let's get rid of these superfluous splnet()/splx() dances.

ok?

Index: netinet/ip_carp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.302
diff -u -p -r1.302 ip_carp.c
--- netinet/ip_carp.c 20 Feb 2017 06:29:42 -0000 1.302
+++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -0000
@@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
 {
  struct ifnet *ifp0;
  struct carp_if *cif;
- int s;
 
  carp_del_all_timeouts(sc);
 
@@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
  /* Restore previous input handler. */
  if_ih_remove(ifp0, carp_input, cif);
 
- s = splnet();
  if (sc->lh_cookie != NULL)
  hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
 
@@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
  free(cif, M_IFADDR, sizeof(*cif));
  }
  sc->sc_carpdev = NULL;
- splx(s);
 }
 
 /* Detach an interface from the carp. */
@@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
  struct carp_if *cif, *ncif = NULL;
  struct carp_softc *vr, *last = NULL, *after = NULL;
  int myself = 0, error = 0;
- int s;
 
  KASSERT(ifp0 != sc->sc_carpdev);
  KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
@@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
  /* Change input handler of the physical interface. */
  if_ih_insert(ifp0, carp_input, cif);
 
- s = splnet();
  carp_carpdev_state(ifp0);
- splx(s);
 
  return (0);
 }

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unneeded splnet()/splx() in carp(4)

Martin Pieuchot
On 07/03/17(Tue) 11:08, Martin Pieuchot wrote:
> carp(4), as a pseudo-interface, is always executed in the 'softnet'
> thread.  Using splnet()/splx() might have been relevant when link-state
> handlers where directly executed from hardware interrupt handlers.  But
> nowadays everything is run under the NET_LOCK() in a thread context, so
> let's get rid of these superfluous splnet()/splx() dances.
>
> ok?

Anyone?

>
> Index: netinet/ip_carp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.302
> diff -u -p -r1.302 ip_carp.c
> --- netinet/ip_carp.c 20 Feb 2017 06:29:42 -0000 1.302
> +++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -0000
> @@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
>  {
>   struct ifnet *ifp0;
>   struct carp_if *cif;
> - int s;
>  
>   carp_del_all_timeouts(sc);
>  
> @@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
>   /* Restore previous input handler. */
>   if_ih_remove(ifp0, carp_input, cif);
>  
> - s = splnet();
>   if (sc->lh_cookie != NULL)
>   hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
>  
> @@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
>   free(cif, M_IFADDR, sizeof(*cif));
>   }
>   sc->sc_carpdev = NULL;
> - splx(s);
>  }
>  
>  /* Detach an interface from the carp. */
> @@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
>   struct carp_if *cif, *ncif = NULL;
>   struct carp_softc *vr, *last = NULL, *after = NULL;
>   int myself = 0, error = 0;
> - int s;
>  
>   KASSERT(ifp0 != sc->sc_carpdev);
>   KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
> @@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
>   /* Change input handler of the physical interface. */
>   if_ih_insert(ifp0, carp_input, cif);
>  
> - s = splnet();
>   carp_carpdev_state(ifp0);
> - splx(s);
>  
>   return (0);
>  }
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unneeded splnet()/splx() in carp(4)

Mike Belopuhov-5
In reply to this post by Martin Pieuchot
On 7 March 2017 at 11:08, Martin Pieuchot <[hidden email]> wrote:
> carp(4), as a pseudo-interface, is always executed in the 'softnet'
> thread.  Using splnet()/splx() might have been relevant when link-state
> handlers where directly executed from hardware interrupt handlers.  But
> nowadays everything is run under the NET_LOCK() in a thread context, so
> let's get rid of these superfluous splnet()/splx() dances.
>
> ok?
>

Looks good to me. Hooray for the last pair of splnets in netinet going the
way of the Dodo.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Unneeded splnet()/splx() in carp(4)

Alexander Bluhm
In reply to this post by Martin Pieuchot
On Tue, Mar 07, 2017 at 11:08:43AM +0100, Martin Pieuchot wrote:
> carp(4), as a pseudo-interface, is always executed in the 'softnet'
> thread.  Using splnet()/splx() might have been relevant when link-state
> handlers where directly executed from hardware interrupt handlers.  But
> nowadays everything is run under the NET_LOCK() in a thread context, so
> let's get rid of these superfluous splnet()/splx() dances.
>
> ok?

OK bluhm@

>
> Index: netinet/ip_carp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.302
> diff -u -p -r1.302 ip_carp.c
> --- netinet/ip_carp.c 20 Feb 2017 06:29:42 -0000 1.302
> +++ netinet/ip_carp.c 7 Mar 2017 10:05:08 -0000
> @@ -898,7 +898,6 @@ carpdetach(struct carp_softc *sc)
>  {
>   struct ifnet *ifp0;
>   struct carp_if *cif;
> - int s;
>  
>   carp_del_all_timeouts(sc);
>  
> @@ -926,7 +925,6 @@ carpdetach(struct carp_softc *sc)
>   /* Restore previous input handler. */
>   if_ih_remove(ifp0, carp_input, cif);
>  
> - s = splnet();
>   if (sc->lh_cookie != NULL)
>   hook_disestablish(ifp0->if_linkstatehooks, sc->lh_cookie);
>  
> @@ -938,7 +936,6 @@ carpdetach(struct carp_softc *sc)
>   free(cif, M_IFADDR, sizeof(*cif));
>   }
>   sc->sc_carpdev = NULL;
> - splx(s);
>  }
>  
>  /* Detach an interface from the carp. */
> @@ -1680,7 +1677,6 @@ carp_set_ifp(struct carp_softc *sc, stru
>   struct carp_if *cif, *ncif = NULL;
>   struct carp_softc *vr, *last = NULL, *after = NULL;
>   int myself = 0, error = 0;
> - int s;
>  
>   KASSERT(ifp0 != sc->sc_carpdev);
>   KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
> @@ -1754,9 +1750,7 @@ carp_set_ifp(struct carp_softc *sc, stru
>   /* Change input handler of the physical interface. */
>   if_ih_insert(ifp0, carp_input, cif);
>  
> - s = splnet();
>   carp_carpdev_state(ifp0);
> - splx(s);
>  
>   return (0);
>  }

Loading...