Index | Thread | Search

From:
Alexandr Nedvedicky <sashan@fastmail.net>
Subject:
Re: UDP PCB table split IPv4 and IPv6
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Wed, 10 Jan 2024 16:58:55 +0100

Download raw body.

Thread
On Wed, Jan 10, 2024 at 02:59:00PM +0100, Alexander Bluhm wrote:
> On Wed, Jan 10, 2024 at 01:30:05AM +0100, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > On Mon, Jan 08, 2024 at 09:59:21AM +0100, Alexander Bluhm wrote:
> > > Hi,
> > >
> > > Currently we have one UDP table for both IPv4 and IPv6 PCB.
> > > I would like to split it like Raw IP and divert tables.
> > >
> > > Then we have less contention on the table mutex when IPv4 and IPv6
> > > are used simultaneously.  Also individual hash tables get smaller
> > > and looping over all PCBs gets shorter.
> > >
> > > ok?
> >
> >     change makes sense, I have no objection. I have one comment/question:
> >
> > </snip>
> > > @@ -847,10 +866,10 @@ udp6_ctlinput(int cmd, struct sockaddr *
> > >  			 */
> > >  		}
> > >
> > > -		in6_pcbnotify(&udbtable, &sa6, uh.uh_dport,
> > > +		in6_pcbnotify(&udb6table, &sa6, uh.uh_dport,
> > >  		    &sa6_src, uh.uh_sport, rdomain, cmd, cmdarg, notify);
> > >  	} else {
> > > -		in6_pcbnotify(&udbtable, &sa6, 0,
> > > +		in6_pcbnotify(&udb6table, &sa6, 0,
> > >  		    &sa6_any, 0, rdomain, cmd, cmdarg, notify);
> > >  	}
> > >  }
> >
> >     hunk above belongs to function udp6_ctlinput(). Are you sure
> >     &udb6table address is correct in else branch? If I understand
> >     things right the else branch matches condition !ip6, so
> >     I would expect &udbtable here.
> 
> The ip6 is the header of the quoted IP packet within the ICMP6
> packet.  If it was IP in ICMP6 the first branch is taken.
> 
> The else branch is called from pfctlinput() without ICMP6 payload.
> But is it still IPv6, it is checked at start of udp6_ctlinput().
> 
>         if (sa == NULL)
>                 return;
>         if (sa->sa_family != AF_INET6 ||
>             sa->sa_len != sizeof(struct sockaddr_in6))
>                 return;
> 

    got it. thanks for clarification.
    feel free to commit with OK sashan@

regards
sashan