On Sat, Apr 04, 2015 at 12:10:30AM +0800, Kevin Lo wrote: > On Fri, Apr 03, 2015 at 08:16:18AM -0700, Ben Pfaff wrote: > > > > On Fri, Apr 03, 2015 at 10:02:16PM +0800, Kevin Lo wrote: > > > I'm attempting to install ovs 2.3.1 on FreeBSD and it appears to be broken > > > after the commit 666afb55e84e9118812de81a75655ec9567b7a5b. > > > Since FreeBSD uses two short integers to represent interface flags, > > > we have to apply mask 0xffff to flags. > > > > > > Signed-off-by: Kevin Lo <ke...@freebsd.org> > > > > CC: Ed Maste <ema...@freebsd.org> > > > It's hard for me to see how your patch makes a difference. If > > ifr->ifr_flags is 16-bits, then ifr->ifr_flags and (ifr->ifr_flags & > > 0xffff) have the same value, and assigning to ifr->ifr_flags has the > > same effect whether you mask off high bits or not. > > FreeBSD fills the int return value with ifr_flagshigh in the high > 16 bits and ifr_flags in the low 16 bits rather than blindly promoting > ifr_flags to an int, which will preserve the sign. > FreeBSD places it into the lower 16 bits of the return value.
You mean that the problem is sign extension in converting short to int, so that if the high bit of ifr_flags is set then ifr_get_flags() returns a corrupted value? OK, I buy that. Can you update the commit message to say so? I don't see how if_set_flags() could be affected by this but it doesn't hurt to update that too. > > Also, the commit you name was in 2013, and we've had other > > contributions from FreeBSD contributors since then > > (namely Ed Maste <ema...@freebsd.org>) and it seems like he > > would have noticed if it were totally busted. > > You mean this message: > http://openvswitch.org/pipermail/dev/2013-May/027594.html > > If so, Ed replied that he reviewed the patch but didn't apply it to test. :-) Ed is the author of the following 37 commits in the Open vSwitch repository. Ed, did you really never test Open vSwitch on FreeBSD, despite this long list of contributions? Ed Maste (37): lib: Do not assume sig_atomic_t is int. vlandev: Move Linux #include under #ifdef __linux__ Route-table implementation for (Free)BSD Add forgotten header Avoid implementation-defined strerror behaviour Use int type for setsockopt IP_TOS value tests: Handle different output formats for 'wc -l'. tests: Make compatible with FreeBSD's sed. Don't assume python is in /usr/bin. tests: Avoid xargs, for FreeBSD compatibility. utilities: FreeBSD compatibility. tests: Get max rx socket buffer size on FreeBSD lib: Correct "old-style function definition" warning. lib: Add header #include for writev netdev-bsd: Initialize variable to silence a compiler warning. netdev: Map to OpenFlow port for flow lookup lib: Add xpipe_nonblocking helper tests: Also enable FreeBSD libc debugging tests: jemalloc debug config for FreeBSD 9 and 10. lib: Move addition of program_name to proctitle_set lib: Use FreeBSD libc function for proctitle lib: Accomodate FreeBSD return value for ssl connection. lib: remove duplicate #include <config.h> Prevent pager from appearing during build netdev-bsd: Use underlying tap device on netdev_bsd_listen(). vswitchd: Avoid writing to const struct netdev: Add NULL get_tunnel_config for BSD. socket-util: restore building on FreeBSD. netdev-bsd: Use UINT64_MAX for unsupported stats. netdev-bsd: Adjust argument line wrapping configure: Add if_mib.h prerequisites. netdev-bsd: Correct pointer use after refactoring. netdev-bsd: Silence warnings on unimplemented platform. lib: Restore build on FreeBSD bfd: Include prerequisite header for FreeBSD tests: Fix build on FreeBSD util: Include pthread_np.h on FreeBSD Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
