Re: promisc/netgraph question

Hello Gleb,

thanks for the patch. I did a quick check and it does not work as expected.

If my understanding is correct (correct me if I'm wrong), the problem is not that M_PROMISC needs to be restored in the same routine.
The problem seems to be that ether_input_internal() is not executed till the end if netgraph (or bridge) claims the frame.
All destination address checks are performed at the end of ether_input_internal() and are skipped if netgraph claims a frame.

Because M_PROMISC is always set to 0 before the frame is being passed to netgraph, and netgraph passes the mbuf to ether_demux(), there is never a chance for M_PROMISC to survive in the chain even if destination address checks were performed and M_PROMISC was set accordingly - it is always unset.
As M_PROMISC together with M_PPROMISC are the only indicators for ether_demux() to drop a frame, this never happens.

Maybe I understand something wrong...?

I think it may be worth thinking about re-ordering the checks or moving them to ether_demux().

--------------------------------------------------
I also did a dirty test with the following. Of course this can't be a solution but it adds an (incomplete) check to ether_demux() and mitigates the issue.

diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index 9c157bf3d..4cfddafd6 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -890,6 +890,10 @@ ether_demux(struct ifnet *ifp, struct mbuf *m)
                return;
        }

+       if (!ETHER_IS_MULTICAST(eh->ether_dhost) &&
+               memcmp(IF_LLADDR(ifp), eh->ether_dhost, ETHER_ADDR_LEN) != 0)
+               m->m_flags |= M_PROMISC;
+
        /*
         * Pass promiscuously received frames to the upper layer if the user
         * requested this by setting IFF_PPROMISC. Otherwise, drop them.

--------------------------------------------------
Just for information how I did the network tests:

sysctl net.inet.ip.forwarding=1

ngctl mkpeer em0: tee lower left && \
ngctl name em0:lower tee1 && \
ngctl connect em0: tee1: upper right

ngctl msg em0: setpromisc 1

--------------------------------------------------

Regards,
Martin