[ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".

Ben Pfaff blp at nicira.com
Fri May 10 20:36:24 UTC 2013


I already applied this on the strength of Ed's review.  However, you
make good points, see below...

On Fri, May 10, 2013 at 01:04:26PM -0700, Ethan Jackson wrote:
> The indentation is newly incorrect in the calls to
> netdev_bsd_do_ioctl(), the prototype for netdev_bsd_update_flags(),
> and the prototype for netdev_vport_update_flags().

I applied Ed's patch to fix that up.

> In netdev_linux_set_etheraddr(), Do we still need to check if
> 'netdev_' is NETDEV_UP before turning it off, or can we skip the check
> and always call netdev_turn_flags_off()?

Doesn't look like the check is needed.  I sent out a fix.

> It's clear how restore_all_flags() is called in the case that
> ovs-vswitchd dies due to a fatal signa.  However, it's not obvious to
> me that it's called when ovs-vswitchd exists gracefully (i.e. due to
> ovs-appctl exit).  Perhaps I'm missing it?

Passing true for the run_at_exit argument to fatal_signal_add_hook()
causes restore_all_flags() to get called from an atexit() handler
installed by fatal_signal_init().

> Out of curiosity, is it that important that we even have this flag
> restoring logic?  Is it a big deal if ovs leaves devices in
> promiscuous mode when it dies?

Restoring flags is probably not necessary.  It only seems polite to me,
though, to try.



More information about the dev mailing list