On Wed, Oct 24, 2018 at 04:34:43PM -0300, Martin Pieuchot wrote:
> I'd like to take brigde_input() & bridge_output() outside of the
> KERNEL_LOCK(). My previous approach relying on the NET_LOCK() didn't
> work because wireless drivers might call bridge_output() from their
> interrupt handler.
>
> So we'll need another mechanism. The first step would be to use a
> mutex. One of the fields that will be protect by this lock is
> `if_bridgeport'. So here's a small cleanup diff to make it clear
> that the field is read in ioctl path.
>
> ok?
OK bluhm@
> Index: net/bridgectl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/bridgectl.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 bridgectl.c
> --- net/bridgectl.c 22 Oct 2018 13:18:23 -0000 1.10
> +++ net/bridgectl.c 24 Oct 2018 19:33:56 -0000
> @@ -55,7 +55,7 @@ int bridge_rtfind(struct bridge_softc *,
> int bridge_rtdaddr(struct bridge_softc *, struct ether_addr *);
> u_int32_t bridge_hash(struct bridge_softc *, struct ether_addr *);
>
> -int bridge_brlconf(struct bridge_softc *, struct ifbrlconf *);
> +int bridge_brlconf(struct bridge_iflist *, struct ifbrlconf *);
> int bridge_addrule(struct bridge_iflist *, struct ifbrlreq *, int out);
>
> int
> @@ -64,6 +64,7 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
> struct bridge_softc *sc = (struct bridge_softc *)ifp->if_softc;
> struct ifbreq *req = (struct ifbreq *)data;
> struct ifbrlreq *brlreq = (struct ifbrlreq *)data;
> + struct ifbrlconf *bc = (struct ifbrlconf *)data;
> struct ifbareq *bareq = (struct ifbareq *)data;
> struct ifbrparam *bparam = (struct ifbrparam *)data;
> struct bridge_iflist *bif;
> @@ -160,7 +161,17 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
> bridge_flushrule(bif);
> break;
> case SIOCBRDGGRL:
> - error = bridge_brlconf(sc, (struct ifbrlconf *)data);
> + ifs = ifunit(bc->ifbrl_ifsname);
> + if (ifs == NULL) {
> + error = ENOENT;
> + break;
> + }
> + bif = (struct bridge_iflist *)ifs->if_bridgeport;
> + if (bif == NULL || bif->bridge_sc != sc) {
> + error = ESRCH;
> + break;
> + }
> + error = bridge_brlconf(bif, bc);
> break;
> default:
> break;
> @@ -535,21 +546,13 @@ bridge_update(struct ifnet *ifp, struct
> * bridge filter/matching rules
> */
> int
> -bridge_brlconf(struct bridge_softc *sc, struct ifbrlconf *bc)
> +bridge_brlconf(struct bridge_iflist *bif, struct ifbrlconf *bc)
> {
> - struct ifnet *ifp;
> - struct bridge_iflist *bif;
> + struct bridge_softc *sc = bif->bridge_sc;
> struct brl_node *n;
> struct ifbrlreq req;
> int error = 0;
> u_int32_t i = 0, total = 0;
> -
> - ifp = ifunit(bc->ifbrl_ifsname);
> - if (ifp == NULL)
> - return (ENOENT);
> - bif = (struct bridge_iflist *)ifp->if_bridgeport;
> - if (bif == NULL || bif->bridge_sc != sc)
> - return (ESRCH);
>
> SIMPLEQ_FOREACH(n, &bif->bif_brlin, brl_next) {
> total++;
