[ovs-dev] [ovs-discuss] WARN_ON_ONCE in dp_output_control

Ian Campbell Ian.Campbell at eu.citrix.com
Tue Sep 1 11:10:57 UTC 2009


On Fri, 2009-08-28 at 17:36 +0100, Ben Pfaff wrote:
> Ian Campbell <Ian.Campbell at citrix.com> writes:
> 
> > I've just updated our internal tree to use current head vswitch
> > (0babc06fb3cd2a6c80d730e3a8dda64cf6298500 "vswitchd: Fix bug in Ethernet
> > address selection for bridge.") and I am seeing the WARN_ON_ONCE at
> > datapath.c:659 triggering. (I might have been seeing it before and not
> > noticed so the fact I've just updated might be irrelevant). The kernel
> > is 2.6.27 based.
> >
> > Is the warning actually necessary or is this a normal (and correctly
> > handled) occurrence?
> 
> I inserted this WARN_ON_ONCE() because at the time I had no way
> to easily test the code on that side of the #if fork: it seemed
> difficult or impossible to trigger the condition on anything but
> Xen, and Xen Dom0 didn't have a new enough kernel (at least as
> far as I know).
> 
> But if you've started seeing this then I should probably commit
> the following.  Comments?

Looks ok to me.

> 
> From 50084c6c3dbd1990dbc29ba192928e1ba72107fc Mon Sep 17 00:00:00 2001
> From: Ben Pfaff <blp at nicira.com>
> Date: Fri, 28 Aug 2009 09:34:19 -0700
> Subject: [PATCH] datapath: Remove WARN_ON_ONCE(1) now that this code path has been exercised.
> 
> The code on one side of this #if fork was difficult to test until Xen
> upgraded to a new enough kernel that it would exercise it.  Later Xen
> kernels are now available and this code path has been tested, at least to
> some extent, so remove the warning.
> 
> Thanks to Ian Campbell <Ian.Campbell at citrix.com> for pointing out the
> warning.
> ---
>  datapath/datapath.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 9d163ea..d822b73 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -646,17 +646,14 @@ dp_output_control(struct datapath *dp, struct sk_buff *skb, int queue_no,
>  
>   	/* If a checksum-deferred packet is forwarded to the controller,
>  	 * correct the pointers and checksum.  This happens on a regular basis
> -	 * only on Xen (the CHECKSUM_HW case), on which VMs can pass up packets
> -	 * that do not have their checksum computed.  We also implement it for
> -	 * the non-Xen case, but it is difficult to trigger or test this case
> -	 * there, hence the WARN_ON_ONCE().
> +	 * only on Xen, on which VMs can pass up packets that do not have their
> +	 * checksum computed.
>   	 */
>  	err = vswitch_skb_checksum_setup(skb);
>   	if (err)
>  		goto err_kfree_skb;
>  #ifndef CHECKSUM_HW
>  	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -		WARN_ON_ONCE(1);
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
>  		/* Until 2.6.22, the start of the transport header was also the
>  		 * start of data to be checksummed.  Linux 2.6.22 introduced





More information about the dev mailing list