On Tue, Dec 14, 2021 at 06:25:20PM +0900, YASUOKA Masahiko wrote:
> Yes, if there is another better idea, it will be welcome.
> For this moment, the diff is the best idea for me.
Sorry, no better idea. I have no experiance with l2pt. Codewise
the diff looks fine, but I don't understand the consequences.
> + if (tdbflow != NULL)
> + rn = rn_lookup((caddr_t)&tdbflow->tdb_filter,
> + (caddr_t)&tdbflow->tdb_filtermask, rnh);
Does rn_lookup() modify the radix tree? I looks like rn_lookup ->
rn_addmask -> rn_insert() does that. This will make it impossible
to make IPsec MP capable. The radix tree is not MP safe, art has
been implemented as an alternative. An ipsp_spd_lookup() should
not modify the flows. It is stange that a function named rn_lookup()
does modifications. Did I miss something?
Why do you call rn_lookup() here? Could we add the masks earlier
when the flows are added?
> + else if (tdbp != NULL)
> + rn = rn_lookup((caddr_t)&tdbp->tdb_filter,
> + (caddr_t)&tdbp->tdb_filtermask, rnh);
What are the consequences of this chunk for regular IPsec?
> /* Match source/dest IDs. */
> - if (ipo->ipo_ids)
> - if (tdbp->tdb_ids == NULL ||
> - !ipsp_ids_match(ipo->ipo_ids,
> tdbp->tdb_ids))
> + if (ipo->ipo_ids != NULL) {
> + if ((tdbp->tdb_flags & TDBF_TUNNELING) == 0 &&
> + (tdbp->tdb_flags & TDBF_UDPENCAP) != 0) {
> + /*
> + * Skip IDs check for transport mode
> + * with NAT-T. Multiple clients (IDs)
> + * can use a same policy.
> + */
> + } else if (tdbp->tdb_ids == NULL &&
> + !ipsp_ids_match(ipo->ipo_ids,
> + tdbp->tdb_ids))
> goto nomatchin;
> + }
This was added to make IPsec/l2tp work in rev 1.85. And now you
change it to make it work. I wish markus@ or mikeb@ could give a
clue.
bluhm
