> On 8 Apr 2016, at 12:58 AM, Martin Pieuchot <[email protected]> wrote:
>
> I'd like to add a lock/unlock the remaining parts of the IPv4 forwarding
> path that still require some work:
>
> 1/ IP source options
> 2/ per-ifp lookups related to multicast traffic
> 3/ IPsec lookups
>
> This should not matter for the moment since the whole path already run
> under the big lock. However this document which part of the code needs
> to be fixed and will allow us to test the routing lookup with PF
> disabled, then the big fat PF_LOCK.
>
> Comments? Oks?
ok
>
> Index: netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 in.c
> --- netinet/in.c 21 Jan 2016 11:23:48 -0000 1.126
> +++ netinet/in.c 7 Apr 2016 14:48:01 -0000
> @@ -890,8 +890,10 @@ in_hasmulti(struct in_addr *ap, struct i
> struct in_multi *inm;
> int joined;
>
> + KERNEL_LOCK();
> IN_LOOKUP_MULTI(*ap, ifp, inm);
> joined = (inm != NULL);
> + KERNEL_UNLOCK();
>
> return (joined);
> }
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.269
> diff -u -p -r1.269 ip_input.c
> --- netinet/ip_input.c 29 Mar 2016 10:34:42 -0000 1.269
> +++ netinet/ip_input.c 7 Apr 2016 14:48:01 -0000
> @@ -227,7 +227,7 @@ ipv4_input(struct mbuf *m)
> {
> struct ifnet *ifp;
> struct ip *ip;
> - int hlen, len;
> + int rv, hlen, len;
> in_addr_t pfrdr = 0;
>
> ifp = if_get(m->m_pkthdr.ph_ifidx);
> @@ -355,8 +355,6 @@ ipv4_input(struct mbuf *m)
>
> #ifdef MROUTING
> if (ipmforwarding && ip_mrouter) {
> - int rv;
> -
> if (m->m_flags & M_EXT) {
> if ((m = m_pullup(m, hlen)) == NULL) {
> ipstat.ips_toosmall++;
> @@ -430,7 +428,10 @@ ipv4_input(struct mbuf *m)
> }
> #ifdef IPSEC
> if (ipsec_in_use) {
> - if (ip_input_ipsec_fwd_check(m, hlen) != 0) {
> + KERNEL_LOCK();
> + rv = ip_input_ipsec_fwd_check(m, hlen);
> + KERNEL_UNLOCK();
> + if (rv != 0) {
> ipstat.ips_cantforward++;
> goto bad;
> }
> @@ -1025,6 +1026,7 @@ ip_dooptions(struct mbuf *m, struct ifne
> cp = (u_char *)(ip + 1);
> cnt = (ip->ip_hl << 2) - sizeof (struct ip);
>
> + KERNEL_LOCK();
> for (; cnt > 0; cnt -= optlen, cp += optlen) {
> opt = cp[IPOPT_OPTVAL];
> if (opt == IPOPT_EOL)
> @@ -1225,12 +1227,14 @@ ip_dooptions(struct mbuf *m, struct ifne
> ipt.ipt_ptr += sizeof(u_int32_t);
> }
> }
> + KERNEL_UNLOCK();
> if (forward && ipforwarding) {
> ip_forward(m, ifp, 1);
> return (1);
> }
> return (0);
> bad:
> + KERNEL_UNLOCK();
> icmp_error(m, type, code, 0, 0);
> ipstat.ips_badoptions++;
> return (1);
> Index: netinet/ip_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.318
> diff -u -p -r1.318 ip_output.c
> --- netinet/ip_output.c 11 Feb 2016 12:56:08 -0000 1.318
> +++ netinet/ip_output.c 7 Apr 2016 14:48:01 -0000
> @@ -100,10 +100,9 @@ ip_output(struct mbuf *m0, struct mbuf *
> struct ifnet *ifp = NULL;
> struct mbuf *m = m0;
> int hlen = sizeof (struct ip);
> - int len, error = 0;
> + int rv, len, error = 0;
> struct route iproute;
> struct sockaddr_in *dst;
> - struct in_ifaddr *ia;
> struct tdb *tdb = NULL;
> u_long mtu;
>
> @@ -183,9 +182,17 @@ reroute:
> if ((IN_MULTICAST(ip->ip_dst.s_addr) ||
> (ip->ip_dst.s_addr == INADDR_BROADCAST)) &&
> imo != NULL && (ifp = if_get(imo->imo_ifidx)) != NULL) {
> + struct in_ifaddr *ia;
> +
> mtu = ifp->if_mtu;
> + KERNEL_LOCK();
> IFP_TO_IA(ifp, ia);
> + if (ip->ip_src.s_addr == INADDR_ANY && ia)
> + ip->ip_src = ia->ia_addr.sin_addr;
> + KERNEL_UNLOCK();
> } else {
> + struct in_ifaddr *ia;
> +
> if (ro->ro_rt == NULL)
> ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
> &ip->ip_src.s_addr, ro->ro_tableid);
> @@ -206,17 +213,19 @@ reroute:
>
> if (ro->ro_rt->rt_flags & RTF_GATEWAY)
> dst = satosin(ro->ro_rt->rt_gateway);
> - }
>
> - /* Set the source IP address */
> - if (ip->ip_src.s_addr == INADDR_ANY && ia)
> - ip->ip_src = ia->ia_addr.sin_addr;
> + /* Set the source IP address */
> + if (ip->ip_src.s_addr == INADDR_ANY && ia)
> + ip->ip_src = ia->ia_addr.sin_addr;
> + }
>
> #ifdef IPSEC
> if (ipsec_in_use || inp != NULL) {
> + KERNEL_LOCK();
> /* Do we have any pending SAs to apply ? */
> tdb = ip_output_ipsec_lookup(m, hlen, &error, inp,
> ipsecflowinfo);
> + KERNEL_UNLOCK();
> if (error != 0) {
> /* Should silently drop packet */
> if (error == -EINVAL)
> @@ -289,9 +298,13 @@ reroute:
> * of outgoing interface.
> */
> if (ip->ip_src.s_addr == INADDR_ANY) {
> + struct in_ifaddr *ia;
> +
> + KERNEL_LOCK();
> IFP_TO_IA(ifp, ia);
> if (ia != NULL)
> ip->ip_src = ia->ia_addr.sin_addr;
> + KERNEL_UNLOCK();
> }
>
> if ((imo == NULL || imo->imo_loop) &&
> @@ -322,8 +335,6 @@ reroute:
> */
> if (ipmforwarding && ip_mrouter &&
> (flags & IP_FORWARDING) == 0) {
> - int rv;
> -
> KERNEL_LOCK();
> rv = ip_mforward(m, ifp);
> KERNEL_UNLOCK();
> @@ -389,8 +400,10 @@ sendit:
> * Check if the packet needs encapsulation.
> */
> if (tdb != NULL) {
> + KERNEL_LOCK();
> /* Callee frees mbuf */
> error = ip_output_ipsec_send(tdb, m, ifp, ro);
> + KERNEL_UNLOCK();
> goto done;
> }
> #endif /* IPSEC */
>
