* Stefan Rompf <[EMAIL PROTECTED]> 2005-11-28 14:15
> Hi,
>
> ok, at least some progress has happened:
Looks good, it would be nice if you could separate the vlan part as a
separate patch for later on though.
> -Replaced device-specific oper_state method with NETIF_F_STACKED flag to
> select between IF_OPER_DOWN or IF_OPER_LOWERLAYERDOWN
> -sysfs support to read operstate
The effort is nice but why do we need sysfs? Isn't netlink enough for
you?
> -completed netlink support (Jamal, Thomas, can you verify the code?)
The code is correct although not using the latest macros. Depending on
whether this patch or my rtnetlink netlink cleanup goes in first we
can decide which ones to use, it's only consistency anyways. ;)
> -added netif_oper_up() query function
> -treat IF_OPER_UNKNOWN equivalent to IF_OPER_UP in some cases to have compat
> to devices that do not set carrier state
> -adopted vlan drivers
> -verified operation with starfire, vlan, loopback. IF_OPER_UNKNOWN won't
> propagate to upper layers when stacking devices.
> +#define IFF_CARRIER 0x10000 /* driver signals carrier */
> +#define IFF_DORMANT 0x20000 /* driver signals dormant */
I would favour another name for IFF_CARRIER such as IFF_LOWERLAYER or
something like that so we can use it to define it to be the status of
the lower layer in general not just of underlying hardware media.
> @@ -80,6 +84,22 @@
> #define IF_PROTO_FR_ETH_PVC 0x200B
> #define IF_PROTO_RAW 0x200C /* RAW Socket */
>
> +/* RFC 2863 operational status */
> +enum {
> + IF_OPER_UNKNOWN,
> + IF_OPER_NOTPRESENT,
Why don't we use this to represent the hardware not being present?
> + IF_OPER_DOWN,
> + IF_OPER_LOWERLAYERDOWN,
> + IF_OPER_TESTING,
> + IF_OPER_DORMANT,
> + IF_OPER_UP,
> +};
I did like your way of leaving gaps to ease inserting new states.
> @@ -308,6 +309,7 @@
> #define NETIF_F_VLAN_CHALLENGED 1024 /* Device cannot handle VLAN
> packets */
> #define NETIF_F_TSO 2048 /* Can offload TCP/IP segmentation */
> #define NETIF_F_LLTX 4096 /* LockLess TX */
> +#define NETIF_F_STACKED 8192 /* Interface is stacked */
Nice idea although I think it's not needed. I disagree with adding a new
feature flag just to please an RFC which is obviously incomplete in this
regard. The purpose of LOWERLAYERDOWN proposed by the RFC is correct but
it can easly be extended to include the hardware carrier without violating
any rules. LOWERLAYERDOWN is a refinement of DOWN so all stacking rules
will continue to work.
> unsigned short padded; /* How much padding added by
> alloc_netdev() */
>
> + unsigned char operstate; /* RFC2863 operstate */
Why do we have to store the operstate?
> + unsigned char link_mode; /* mapping policy to operstate */
We can't use flags for this? Do we really need a new field just to
mark an interface to go dormant?
> +
> unsigned mtu; /* interface MTU value */
> unsigned short type; /* interface hardware type */
> unsigned short hard_header_len; /* hardware hdr length
> */
> @@ -343,6 +345,26 @@
> new_dev->do_ioctl = vlan_dev_ioctl;
> }
>
> +void vlan_transfer_operstate(const struct net_device *dev, struct net_device
> *vlandev)
> +{
> + if (netif_carrier_ok(dev)) {
> + if (!netif_carrier_ok(vlandev))
> + netif_carrier_on(vlandev);
> + } else {
> + if (netif_carrier_ok(vlandev))
> + netif_carrier_off(vlandev);
> + }
> +
> + /* Have to respect userspace enforced dormant state
> + * of real device, also must allow supplicant running
> + * on VLAN device
> + */
> + if (dev->operstate == IF_OPER_DORMANT)
> + netif_dormant_on(vlandev);
> + else
> + netif_dormant_off(vlandev);
Dormant state should be handled before announcing the carrier or
UP state is leaked for a moment.
> diff -X dontdiff -ur linux-2.6.14/net/8021q/vlan_dev.c
> linux-2.6.14-rfc2863/net/8021q/vlan_dev.c
> --- linux-2.6.14/net/8021q/vlan_dev.c 2005-11-02 11:08:11.000000000 +0100
> +++ linux-2.6.14-rfc2863/net/8021q/vlan_dev.c 2005-11-28 13:56:33.000000000
> +0100
> @@ -786,6 +786,8 @@
> {
> if (!(VLAN_DEV_INFO(dev)->real_dev->flags & IFF_UP))
> return -ENETDOWN;
> + vlan_transfer_operstate(VLAN_DEV_INFO(dev)->real_dev, dev);
> + linkwatch_fire_event(dev); /* force event to setup operstate */
Shouldn't this be handled by vlan_transfer_operstate() already?
> diff -X dontdiff -ur linux-2.6.14/net/core/rtnetlink.c
> linux-2.6.14-rfc2863/net/core/rtnetlink.c
> --- linux-2.6.14/net/core/rtnetlink.c 2005-11-02 11:08:12.000000000 +0100
> +++ linux-2.6.14-rfc2863/net/core/rtnetlink.c 2005-11-28 13:33:31.000000000
> +0100
> @@ -178,6 +178,31 @@
> }
>
>
> +static void set_operstate(struct net_device *dev, unsigned char transition) {
> + unsigned char operstate = dev->operstate;
> + ASSERT_RTNL();
> +
> + switch(transition) {
> + case IF_OPER_UP:
> + if (operstate == IF_OPER_DORMANT ||
> + operstate == IF_OPER_UNKNOWN)
> + operstate = IF_OPER_UP;
> + break;
> + case IF_OPER_DORMANT:
> + if (operstate == IF_OPER_UP ||
> + operstate == IF_OPER_UNKNOWN)
> + operstate = IF_OPER_DORMANT;
> + break;
> + }
> +
> + if (dev->operstate != operstate) {
> + write_lock_bh(&dev_base_lock);
> + dev->operstate = operstate;
> + write_unlock_bh(&dev_base_lock);
> + netdev_state_change(dev);
> + }
Now this is really racy.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
