Hello,
your change looks good to me. Though I have a comment/question to your diff.
> Index: net/if_vxlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 if_vxlan.c
> --- net/if_vxlan.c 25 Oct 2017 09:24:09 -0000 1.63
> +++ net/if_vxlan.c 8 Nov 2017 14:49:58 -0000
> @@ -611,6 +611,7 @@ vxlan_lookup(struct mbuf *m, struct udph
> vni = VXLAN_VNI_UNSET;
> }
>
> + NET_ASSERT_LOCKED();
> /* First search for a vxlan(4) interface with the packet's VNI */
> LIST_FOREACH(sc, &vxlan_tagh[VXLAN_TAGHASH(vni)], sc_entry) {
> if ((uh->uh_dport == sc->sc_dstport) &&
> Index: net/pf.c
I think we are approaching point, which requires us to revisit
NET_ASSERT_LOCKED(). The assert currently tests caller is writer
on net_lock.
Since we are going to 'soften' the NET_LOCK() to R-lock for
some tasks/threads the NET_ASSERT_LOCKED() will become invalid
(false positive) assertion for functions, which are being grabbed
occasionally as readers and other times as writers (?typically in
local outbound path?). I've seen such smoke already in my diffs
I'm working on currently.
So I'd like to know, what's the plan for NET_ASSERT_LOCKED() macro?
a) are we going to relax it, so it will test if the net_lock is
locked?
b) are we going to keep it, but a new 'soft' assert will be introduced
e.g. NET_ASSERT_ALOCKED() (A for any lock)?
c) add parameter to current NET_ASSERT_LOCKED() stating desired
lock level: 0 - unlocked, 1 - reader, 2 - writer
I'm leaning towards option b) introduce a new assertion for cases
where we require lock, but we don't care if it is reader/writer.
As I've said the patch looks good to me as-is and should go in. I just
would like to hear some greater plan for NET_ASSERT_LOCKED(). Perhaps
we should go for it before we will be further playing with parallel
softnet.
thanks a lot
regards
sasha
