On Mon, Feb 13, 2012 at 07:07:31PM -0800, Ethan Jackson wrote:
> When a netdev Linux device is created or its netlink cache is
> invalidate, it needs an alternative method to update the its
> carrier status. Previous patches retrieved this information from a
> sysfs file. This patch switches to ioctl which is significantly
> simpler, and likely quite a bit faster as well.
>
> Signed-off-by: Ethan Jackson <[email protected]>
I did a little research on why this is implemented the way it is. It
dates back to this commit from April 2009:
From: Ben Pfaff <[email protected]>
Date: Wed, 8 Apr 2009 10:36:59 -0700
Subject: [PATCH] secchan: Fix OFPPS_LINK_DOWN detection.
netdev_get_flags() was supposed to return NETDEV_CARRIER if carrier was
detected by the network device PHY, by checking for the IFF_LOWER_UP bit
in the device flags returned by the SIOCGIFFLAGS ioctl. Unfortunately,
IFF_LOWER_UP has value 0x10000 and that ioctl returns a short int, so this
bit was always read as 0, indicating that carrier was off.
There are at least two other ways to get the carrier status. One is via
rtnetlink with RTM_GETLINK. Unfortunately that is only supported on Linux
2.6.19 and up. So we fall back to the other possibility, which is
/sys/net/class/<device>/carrier. I hope that our users mount sysfs.
In other words, I was taking "carrier" very literally, interpreting it
as the Linux feature that directly reports carrier status. But
IFF_RUNNING seems to be just as good for the intended purpose, since
it will not be set if carrier is down. So your patch seems like a
nice improvement to me. It actually makes things more consistent,
since from the patch it appears that we used get_carrier_via_sysfs()
in some cases and IFF_RUNNING in others (via the 'running' member of
struct rtnetlink_link_change).
Your change looks good to me. Can we switch to using an "unsigned
int" for ifi_flags though? That's the type the kernel uses in
ifinfomsg and generally unsigned types are better for bit operations.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
