tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

ETHERCAP_* & ioctl()



 I'd like to make ETHERCAP_* specification clear before fixing PR#38871
for bge(4).

0. Background.

   When changing some flags of a interface, if_init() may be called
  to reflect the change. It's depend on a implementation of each driver.
  It's important not to call if_init() if it's not needed because many
  Ethernet driver's if_init() cause linkdown. It's bad for vrrp.

   For Ethernet, Three flags are related. if_flags, if_capenable and
  ec_capenable.

   Currently, many ioctls are centralized into if_ethersubr.c::ether_ioctl()
  to reduce code duplication.

1. about a new ioctl and callbacks.

 (For IFF_)
 0) only if_flags is available.
 1) some flags are set/unset automatically.
 2) the SOICSIFFLAGS is used to change the value and ec_ifflags_cb can be
    used to reflect the change.

 For IFCAP_,
 0) the if_capabilities and if_capenable entry are separated.
 1) (almost?) all capability flags in if_capenable aren't set automatically.
 2) the SIOCSIFCAP ioctl is used to change the value.

 For ETHERCAP_,
 0) the ec_capabilities and ec_capenable entry are separated, too.
 1) currently, ETHERCAP_VLAN_MTU is set automatically, but
    ETHERCAP_VLAN_HWTAGGING isn't. This is inconsistent.
    For ETHERCAP_VLAN_HWTAGGING, it's not need to on/off the value.
    IMHO, for or allmost all Ethernet driver, it's ok to set the flag in
    the attach function though it's inconsistent.
 2) when ETHERCAP_* is changed, the SIOCSIFFLAGS ioctl is used to change
    the hardware setting in if_vlan.c. The intention is to call if_init()
    of the parent interface. I think this is abuse (see the following table).
    One of the solutions is to introduce the SIOCSETHERCAP ioctl.

 See the following table. This is the current implementation. There is no way
to tell the driver a change of ec_capenable.

   +-----------------+--------------+---------------+
   | ioctl command   | flag         | callback      |
   +-----------------+--------------+---------------+
   | SIOCSIFFLAGS    | if_flags     | ec_ifflags_cb |
   | SIOCSIFCAP      | if_capenable |               |
   |                 | ec_capenable |               |
   +-----------------+--------------+---------------+

 My proposal is:

 - Introduce the SIOCSETHERCAP ioctl command (The macro will be added
   into if_ether.h), ec_ifcap_cb and ec_eccap_cb.

   +-----------------+--------------+---------------+
   | ioctl command   | flag         | callback      |
   +-----------------+--------------+---------------+
   | SIOCSIFFLAGS    | if_flags     | ec_ifflags_cb |
   | SIOCSIFCAP      | if_capenable | (ec_ifcap_cb) |
   | (SIOCSETHERCAP) | ec_capenable | (ec_eccap_cb) |
   +-----------------+--------------+---------------+

 - Use each flags as follows:

   +------------------------+-------------+
   | flag name              | auto on/off |
   |                        | ec_capenable|
   +------------------------+-------------+
   |ETHERCAP_VLAN_MTU       | yes(*1)     |
   |ETHERCAP_VLAN_HWTAGGING | no          |
   |ETHERCAP_JUMBO_MTU      | yes(*2)     |
   +------------------------+-------------+

   (*1) Set if one or more vlans are configured.
   (*2) Set if the mtu is set more than 1500.

   Originally, there is no need to use ec_capenable for ETHERCAP_VLAN_MTU
  and ETHERCAP_JUMBO_MTU. If we use ec_capabilities for those flags, some
  code may have a bug by checking ec_capenable instead of ec_capabilities
  because it's inconsistent with if_cap*.

2. registeration of callbacks.

  Currently ec_ifflags_cb is set as follows:

        ether_ifattach(ifp, eaddr);
        ether_set_ifflags_cb(&sc->ethercom, bge_ifflags_cb);

 We have 2 (or more?) solutions

   A)
        ether_ifattach(ifp, eaddr);
        ether_set_ifflags_cb(&sc->ethercom, bge_ifflags_cb);
        ether_set_ifcap_cb(&sc->ethercom, bge_ifcap_cb);
        ether_set_eccap_cb(&sc->ethercom, bge_eccap_cb);

   B)
        ether_ifattach(ifp, eaddr, bge_ifflags_cb, bge_ifcap_cb, bge_eccap_cb);

   C)
        Any other solutions.

 I've alread implemented with A)

Any suggestion, advice or objection are welcome.

----------------------------------------------------------
                SAITOH Masanobu (masanobu%iij.ad.jp@localhost
                                  msaitoh%netbsd.org@localhost)


Home | Main Index | Thread Index | Old Index