[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