[ovs-dev] [PATCH 3/3] auto-attach: Add auto-attach support to bridge layer and command set

Flynn, Dennis R (Dennis) drflynn at avaya.com
Thu Feb 26 19:49:53 UTC 2015


Hi Ben,

We have completed our testing with your additional refinements and everything looks good.

I have the following comments after reviewing your changes.
Once these are addressed to your satisfaction we are good to go with committing these changes.

lib/lldp/lldp.c

	- Remove lingering reference to POKE macro in comment
	- Should lldp_tlv_end() check for null after call to ofputbuf_at()?
	- lldp_send() - missing statement to set new h_lport.p_lastframe

Update Copyright statements adding '2015' in the following

	- aa-structs.h
	- lib/lldp/lldp.c
	- lib/lldp/lldpd-structs.c
	- lib/lldp/lldpd.c
	- lib/lldp/lldpd.h
	- lib/ovs-lldpd.c  (Windriver)

Thanks,
Dennis


-----Original Message-----
From: Ben Pfaff [mailto:blp at nicira.com] 
Sent: Monday, February 23, 2015 12:21 AM
To: Flynn, Dennis R (Dennis)
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH 3/3] auto-attach: Add auto-attach support to bridge layer and command set

On Sun, Feb 22, 2015 at 11:45:29AM -0800, Ben Pfaff wrote:
> On Fri, Feb 20, 2015 at 02:17:11PM -0500, drflynn at avaya.com wrote:
> > From: Dennis Flynn <drflynn at avaya.com>
> > 
> > This is the final commit in the series of commits that deliver 
> > initial support for Auto-Attach. Specifically this commit delivers 
> > auto-attach support to the OVS bridge layer as well as the new 
> > auto-attach commands. The OVSDB schema is modified to define the new 
> > auto-attach entries. The man pages, unit tests, and news and license 
> > notice files are also updated. A unit test is provided to validate the construction of auto-attach packets.
> > 
> > Signed-off-by: Ludovic Beliveau <ludovic.beliveau at windriver.com>
> > Signed-off-by: Dennis Flynn <drflynn at avaya.com>
> 
> Hi Dennis, thanks for the new version.
> 
> I noticed some new warnings in this version.  These (from "sparse") 
> probably mean that you should mark these variables "static":
> 
>     ../tests/test-aa.c:29:6: warning: symbol 'chassis_mac' was not declared. Should it be static?
>     ../tests/test-aa.c:30:9: warning: symbol 'eth_src' was not declared. Should it be static?
>     ../tests/test-aa.c:36:5: warning: symbol 'num_tests' was not declared. Should it be static?
> 
> Also these from GCC:
> 
>     ../utilities/ovs-vsctl.c: In function ‘cmd_get_aa_mapping’:
>     ../utilities/ovs-vsctl.c:2702:27: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=]
>                                (long int) br->br_cfg->auto_attach->value_mappings[i]
>                                ^
>     ../utilities/ovs-vsctl.c:2702:27: error: format ‘%lld’ expects 
> argument of type ‘long long int’, but argument 4 has type ‘long int’ 
> [-Werror=format=]

Never mind, I took care of this myself and posted a series that includes these and some other refinements.  Dennis, will you look it over?

The series starts here:
        https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_pipermail_dev_2015-2DFebruary_051568.html&d=AwIDaQ&c=BFpWQw8bsuKpl1SgiZH64Q&r=GATBXyTOGTB-HJaugihh79u62rJIdBqLbpVug642mjk&m=5z8G_koRAfQNiuwG9ENu7oavg7XoV7RjU_CuLaNP0Zw&s=d63Ycj6CneN2fwPKi71M7Vo72P3DBvRKJxNkTkKeKa8&e= 

Thanks,

Ben.


More information about the dev mailing list