On Fri, 8 Oct 2004, Giorgos Keramidas wrote: > Are we allowed to change the prototype of ether_demux()? > > If yes, we could make ether_demux() return -1 if it frees the mbuf, and > avoid calling random_harvest() on that particular frame. Note that I > haven't had a chance to test build or run the following patch yet, but > if anyone has a better idea or comments that would help us improve it, > they're welcome. > > One note that I have taken down while writing this and I have to check > is the possibility of calling random_harvest(m->m_data, 16, ...) with an > mbuf whose m->m_data contains less than 16 bytes. I looked at a couple of things: - Move the harvesting before the call to ether_demux() - Delete the harvesting call I haven't yet tried to make the harvesting call use real mbuf data, but I'm somewhat dubious of the real value of that: Bill Simpson has pointed out that in practice most hosts talk primarily to their default gateway or a very small set of local peers. In interrupt-driven operation, you're already reaping entropy from receiving the interrupt, so in that scenario, I think there are diminishing returns from trying to also reap entropy at the ether_demux() point. In the netperf branch, I've simply deleted it -- that said, in polling operation, you don't get network interrupts so there may be some benefit here for that. In the netperf branch, I also have a number of changes to substantially reduce locking overhead in the entropy code. I had hoped to merge those before my recent move, but due to the normal chaos of moving, didn't get to it. I'll prepare a candidate patch and post it in the next couple of days. It both coalesces some mutexes, and attempts to coalesce mutex acquisition when doing work to avoid thrashing mutexes. I dropped a copy to Mark, who's also running with it pretty successfully, and running it on several machines. One thing it does require to be more efficient is O(1) list movement operations to move a list of entries from one queue head to another; right now, I think I'm doing O(n) list walks, which is probably OK in the entropy worker thread due to the short lengths of the queues, but otherwise undesirable. Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert_at_fledge.watson.org Principal Research Scientist, McAfee Research > > --- patch begins here --- > Index: ethernet.h > =================================================================== > RCS file: /home/ncvs/src/sys/net/ethernet.h,v > retrieving revision 1.24 > diff -u -u -r1.24 ethernet.h > --- ethernet.h 5 Oct 2004 19:28:52 -0000 1.24 > +++ ethernet.h 8 Oct 2004 09:15:07 -0000 > _at__at_ -353,7 +353,7 _at__at_ > > extern uint32_t ether_crc32_le(const uint8_t *, size_t); > extern uint32_t ether_crc32_be(const uint8_t *, size_t); > -extern void ether_demux(struct ifnet *, struct mbuf *); > +extern int ether_demux(struct ifnet *, struct mbuf *); > extern void ether_ifattach(struct ifnet *, const u_int8_t *); > extern void ether_ifdetach(struct ifnet *); > extern int ether_ioctl(struct ifnet *, int, caddr_t); > Index: if_ethersubr.c > =================================================================== > RCS file: /home/ncvs/src/sys/net/if_ethersubr.c,v > retrieving revision 1.177 > diff -u -u -r1.177 if_ethersubr.c > --- if_ethersubr.c 27 Jul 2004 23:20:45 -0000 1.177 > +++ if_ethersubr.c 8 Oct 2004 09:16:49 -0000 > _at__at_ -614,16 +614,14 _at__at_ > } > } > > - ether_demux(ifp, m); > - /* First chunk of an mbuf contains good entropy */ > - if (harvest.ethernet) > - random_harvest(m, 16, 3, 0, RANDOM_NET); > + if (ether_demux(ifp, m) == 0 && harvest.ethernet) > + random_harvest(m->m_data, 16, 3, 0, RANDOM_NET); > } > > /* > * Upper layer processing for a received Ethernet packet. > */ > -void > +int > ether_demux(struct ifnet *ifp, struct mbuf *m) > { > struct ether_header *eh; > _at__at_ -666,14 +664,14 _at__at_ > IFP2AC(ifp)->ac_enaddr, ETHER_ADDR_LEN) != 0 > && (ifp->if_flags & IFF_PPROMISC) == 0) { > m_freem(m); > - return; > + return (-1); > } > } > > /* Discard packet if interface is not up */ > if ((ifp->if_flags & IFF_UP) == 0) { > m_freem(m); > - return; > + return (-1); > } > if (ETHER_IS_MULTICAST(eh->ether_dhost)) { > if (bcmp(etherbroadcastaddr, eh->ether_dhost, > _at__at_ -691,7 +689,7 _at__at_ > if (ether_ipfw_chk(&m, NULL, &rule, 0) == 0) { > if (m) > m_freem(m); > - return; > + return (-1); > } > } > #endif > _at__at_ -709,7 +707,7 _at__at_ > */ > KASSERT(vlan_input_p != NULL,("ether_input: VLAN not loaded!")); > (*vlan_input_p)(ifp, m); > - return; > + return (-1); > } > > /* > _at__at_ -725,7 +723,7 _at__at_ > ifp->if_noproto++; > m_freem(m); > } > - return; > + return (-1); > } > > /* Strip off Ethernet header. */ > _at__at_ -749,7 +747,7 _at__at_ > if (ifp->if_flags & IFF_NOARP) { > /* Discard packet if ARP is disabled on interface */ > m_freem(m); > - return; > + return (-1); > } > isr = NETISR_ARP; > break; > _at__at_ -805,7 +803,7 _at__at_ > goto discard; > } > netisr_dispatch(isr, m); > - return; > + return 0; > > discard: > /* > _at__at_ -820,9 +818,10 _at__at_ > */ > M_PREPEND(m, ETHER_HDR_LEN, M_DONTWAIT); > (*ng_ether_input_orphan_p)(ifp, m); > - return; > + return 0; > } > m_freem(m); > + return (-1); > } > > /* > --- patch ends here --- > _______________________________________________ > freebsd-current_at_freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org" >Received on Fri Oct 08 2004 - 09:48:22 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:16 UTC