[ovs-discuss] BUG: Open vSwitch 0.90.4 can't be used with VLANs

Ben Pfaff blp at nicira.com
Fri Sep 11 22:49:54 UTC 2009


Thank you for the patch.  Comments inline:

Jean Tourrilhes <jt at hpl.hp.com> writes:

> --- openvswitch-0.90.4/datapath/datapath.c	2009-07-28 10:32:16.000000000 -0700
> +++ openvswitch-0.90.4-jean-1/datapath/datapath.c	2009-09-10 15:39:33.000000000 -0700
> @@ -177,7 +177,8 @@ static void dp_ifinfo_notify(int event, 
>  		kfree_skb(skb);
>  		goto errout;
>  	}
> -	err = rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
> +	/* Jean II : returns void in 2.6.30 */
> +	rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
>  errout:
>  	if (err < 0)
>  		rtnl_set_sk_err(net, RTNLGRP_LINK, err);

This was already fixed on the openvswitch "master" branch.  I
guess you must be working from the "citrix" branch?  I don't want
to apply it there because we are not attempting to support
kernels newer than 2.6.18 on that branch.

> @@ -1550,6 +1551,7 @@ static int __init dp_init(void)
>  
>  	printk("Open vSwitch %s, built "__DATE__" "__TIME__"\n", VERSION BUILDNR);
>  
> +#ifdef BUG_VLAN
>  	/* Register to receive STP packets because the bridge module also
>  	 * attempts to do so.  Since there can only be a single listener for a
>  	 * given protocol, this provides mutual exclusion against the bridge
> @@ -1560,6 +1562,7 @@ static int __init dp_init(void)
>  		printk(KERN_ERR "openvswitch: can't register sap for STP (probably the bridge module is loaded)\n");
>  		return -EADDRINUSE;
>  	}
> +#endif	/* BUG_VLAN */
>  
>  	err = flow_init();
>  	if (err)
> @@ -1594,7 +1597,9 @@ static void dp_cleanup(void)
>  	unregister_netdevice_notifier(&dp_device_notifier);
>  	flow_exit();
>  	br_handle_frame_hook = NULL;
> +#ifdef BUG_VLAN
>  	llc_sap_put(dp_stp_sap);
> +#endif	/* BUG_VLAN */
>  }
>  
>  module_init(dp_init);

This isn't very useful because you didn't add anything that
defines BUG_VLAN.  We can do better.  Shortly I will send out a
patch that fixes this a different way.

> diff -u -p -r -x '*~' openvswitch-0.90.4/lib/vconn.c openvswitch-0.90.4-jean-1/lib/vconn.c
> --- openvswitch-0.90.4/lib/vconn.c	2009-07-28 10:32:16.000000000 -0700
> +++ openvswitch-0.90.4-jean-1/lib/vconn.c	2009-09-10 16:49:07.000000000 -0700
> @@ -1247,7 +1247,7 @@ check_action(const union ofp_action *a, 
>  {
>      int error;
>  
> -    switch (a->type) {
> +    switch (ntohs(a->type)) {
>      case OFPAT_OUTPUT:
>          error = check_action_port(ntohs(a->output.port), max_ports);
>          if (error) {

Ugh, that's a bad one, thank you for pointing it out.  I pushed
this to the "citrix" branch.  I'll try to merge citrix into
master soon.




More information about the discuss mailing list