On Thu, Jun 23, 2016 at 00:38 +0200, Alexander Bluhm wrote: > On Wed, Jun 22, 2016 at 08:15:09PM +0200, Mike Belopuhov wrote: > > Can you or benno test NAT64 with this change? > > In case of weird behavior do this: > > > > int sidx = pd->af == pd->naf ? pd->sidx : pd->didx; > > int didx = pd->af == pd->naf ? pd->didx : pd->sidx; > > > > And use sidx/didx throughout instead of pd->sidx and pd->didx. > > > > I'm pretty sure you need to do this trick, but I'm not 100% > > certain. > > af-to state lookup in pf_get_sport() is quite broken. > > Jun 23 00:25:26 q70 /bsd: pf: af-to inet6 rdr, 10.188.70.17:3003 -> > 10.188.216.114:7 > Jun 23 00:25:26 q70 /bsd: pf: find state all dir=out, af=24, key0: > fdd7:e83e:66bc:211:725f:caff:fe21:8d70[10001], key1: abc:d872::[7], proto=17 > Jun 23 00:25:26 q70 /bsd: pf: af-to inet6 rdr done, prefixlen 120, > fdd7:e83e:66bc:211:725f:caff:fe21:8d70[10001] -> > fdd7:e83e:66bc:212:725f:caff:fe21:8d72[7] > > Look at the key1: abc:d872::[7], that is the IPv4 address used as IPv6. > pf_get_transaddr_af() will fix the prefix later. >
Looks like the pd->ndaddr/nsaddr patching should happen before calling pf_get_sport. > As there is more work to be done for af-to, I propose this version > of the nat-to fix. With the explicit variables sidx and didx we > can swap it easily if we will need it. > Sure, OK mikeb. > bluhm > > Index: net/pf_lb.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_lb.c,v > retrieving revision 1.53 > diff -u -p -r1.53 pf_lb.c > --- net/pf_lb.c 15 Jun 2016 11:36:06 -0000 1.53 > +++ net/pf_lb.c 22 Jun 2016 22:18:30 -0000 > @@ -155,6 +155,9 @@ pf_get_sport(struct pf_pdesc *pd, struct > struct pf_state_key_cmp key; > struct pf_addr init_addr; > u_int16_t cut; > + int dir = (pd->dir == PF_IN) ? PF_OUT : PF_IN; > + int sidx = pd->sidx; > + int didx = pd->didx; > > bzero(&init_addr, sizeof(init_addr)); > if (pf_map_addr(pd->naf, r, &pd->nsaddr, naddr, &init_addr, sn, &r->nat, > @@ -182,9 +185,9 @@ pf_get_sport(struct pf_pdesc *pd, struct > key.af = pd->naf; > key.proto = pd->proto; > key.rdomain = pd->rdomain; > - PF_ACPY(&key.addr[0], &pd->ndaddr, key.af); > - PF_ACPY(&key.addr[1], naddr, key.af); > - key.port[0] = pd->ndport; > + PF_ACPY(&key.addr[didx], &pd->ndaddr, key.af); > + PF_ACPY(&key.addr[sidx], naddr, key.af); > + key.port[didx] = pd->ndport; > > /* > * port search; start random, step; > @@ -194,20 +197,20 @@ pf_get_sport(struct pf_pdesc *pd, struct > pd->proto == IPPROTO_ICMP || pd->proto == IPPROTO_ICMPV6)) { > /* XXX bug: icmp states dont use the id on both > * XXX sides (traceroute -I through nat) */ > - key.port[1] = pd->nsport; > - if (pf_find_state_all(&key, PF_IN, NULL) == NULL) { > + key.port[sidx] = pd->nsport; > + if (pf_find_state_all(&key, dir, NULL) == NULL) { > *nport = pd->nsport; > return (0); > } > } else if (low == 0 && high == 0) { > - key.port[1] = pd->nsport; > - if (pf_find_state_all(&key, PF_IN, NULL) == NULL) { > + key.port[sidx] = pd->nsport; > + if (pf_find_state_all(&key, dir, NULL) == NULL) { > *nport = pd->nsport; > return (0); > } > } else if (low == high) { > - key.port[1] = htons(low); > - if (pf_find_state_all(&key, PF_IN, NULL) == NULL) { > + key.port[sidx] = htons(low); > + if (pf_find_state_all(&key, dir, NULL) == NULL) { > *nport = htons(low); > return (0); > } > @@ -223,16 +226,16 @@ pf_get_sport(struct pf_pdesc *pd, struct > cut = arc4random_uniform(1 + high - low) + low; > /* low <= cut <= high */ > for (tmp = cut; tmp <= high; ++(tmp)) { > - key.port[1] = htons(tmp); > - if (pf_find_state_all(&key, PF_IN, NULL) == > + key.port[sidx] = htons(tmp); > + if (pf_find_state_all(&key, dir, NULL) == > NULL && !in_baddynamic(tmp, pd->proto)) { > *nport = htons(tmp); > return (0); > } > } > for (tmp = cut - 1; tmp >= low; --(tmp)) { > - key.port[1] = htons(tmp); > - if (pf_find_state_all(&key, PF_IN, NULL) == > + key.port[sidx] = htons(tmp); > + if (pf_find_state_all(&key, dir, NULL) == > NULL && !in_baddynamic(tmp, pd->proto)) { > *nport = htons(tmp); > return (0);