5.3-RELEASE TODO

Robert Watson rwatson at freebsd.org
Fri Oct 8 04:48:22 PDT 2004


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
> @@ -353,7 +353,7 @@
>  
>  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
> @@ -614,16 +614,14 @@
>  		}
>  	}
>  
> -	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;
> @@ -666,14 +664,14 @@
>  		      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,
> @@ -691,7 +689,7 @@
>  		if (ether_ipfw_chk(&m, NULL, &rule, 0) == 0) {
>  			if (m)
>  				m_freem(m);
> -			return;
> +			return (-1);
>  		}
>  	}
>  #endif
> @@ -709,7 +707,7 @@
>  		 */
>  		KASSERT(vlan_input_p != NULL,("ether_input: VLAN not loaded!"));
>  		(*vlan_input_p)(ifp, m);
> -		return;
> +		return (-1);
>  	}
>  
>  	/*
> @@ -725,7 +723,7 @@
>  			ifp->if_noproto++;
>  			m_freem(m);
>  		}
> -		return;
> +		return (-1);
>  	}
>  
>  	/* Strip off Ethernet header. */
> @@ -749,7 +747,7 @@
>  		if (ifp->if_flags & IFF_NOARP) {
>  			/* Discard packet if ARP is disabled on interface */
>  			m_freem(m);
> -			return;
> +			return (-1);
>  		}
>  		isr = NETISR_ARP;
>  		break;
> @@ -805,7 +803,7 @@
>  		goto discard;
>  	}
>  	netisr_dispatch(isr, m);
> -	return;
> +	return 0;
>  
>  discard:
>  	/*
> @@ -820,9 +818,10 @@
>  		 */
>  		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"
> 



More information about the freebsd-current mailing list