* Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> [2020-11-05 09:24]:
> Hello Ralf,
> 
> On Thu, Nov 05, 2020 at 07:59:18AM +0100, Ralf Horstmann wrote:
> > Hi Sashan,
> > 
> > * Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> [2020-11-05 00:25]:
> > > Hello Ralf,
> > > 
> > > I think we should rather give a try diff below. I would rather add
> > > a return after m_freem(). Will diff below work for you too?
> > 
> > Without understanding the full background I went with ignoring the return 
> > value
> 
>     I'm sorry I have not provide better explanation in my first email.
>     this is the code after applying diff I've suggested earlier:
> 
> 2277 void
> 2278 carp_transmit(struct carp_softc *sc, struct ifnet *ifp0, struct mbuf *m)
> 2279 {
> 2280         struct ifnet *ifp = &sc->sc_if;
> 2281 
> 2282 #if NBPFILTER > 0
> 2283         {
> 2284                 caddr_t if_bpf = ifp->if_bpf;
> 2285                 if (if_bpf) {
> 2286                         if (bpf_mtap_ether(if_bpf, m, 
> BPF_DIRECTION_OUT)) {
> 2287                                 m_freem(m);
> 2288                                 return;
> 2289                         }
> 2290                 }
> 2291         }
> 2292 #endif /* NBPFILTER > 0 */
> 2293 
> 2294         if (!ISSET(ifp0->if_flags, IFF_RUNNING)) {
> 2295                 counters_inc(ifp->if_counters, ifc_oerrors);
> 2296                 m_freem(m);
> 2297                 return;
> 2298         }
> 
> 
>     the proposed change adds a 'return;' to line 2288, which is also 
> consistent
>     with code, we can see at following line 2297.

Yes, that part is clear, there either needs to be a return or the bpf return
code needs to be ignored and the m_freem removed.

I'm just wondering why other callers of bpf_mtap_ether with BPF_DIRECTION_OUT
across the tree are ignoring the return code, and why we need to observe it
in carp. E.g. if_vlan.c:

    void
    vlan_transmit(struct vlan_softc *sc, struct ifnet *ifp0, struct mbuf *m)
    {
        struct ifnet *ifp = &sc->sc_if;
        int txprio = sc->sc_txprio;
        uint8_t prio;
    
    #if NBPFILTER > 0
        if (ifp->if_bpf)
             bpf_mtap_ether(ifp->if_bpf, m, BPF_DIRECTION_OUT);
    #endif /* NBPFILTER > 0 */

Since I somehow managed to trigger the case that the drop action was returned
from bpf, I'm suspecting that with your patch packets would be dropped that
should rather be sent out. Maybe the dhcpd installed BPF code needs to be
adjusted as well then.

Regards,
Ralf

Reply via email to