Re: [RFC PATCH 0/3 V3] add DCB support to NetworkManager



On 07/30/2012 12:57 PM, Dan Williams wrote:
On Mon, 2012-07-02 at 17:59 +0800, Weiping Pan wrote:
This patchset is to add DCB support to NetworkManager.
Its function is like dcbtool, and it will communicate with lldpad through
liblldp_clif shared library.
One question I've got is about when the DCB stuff gets run.  The current
patch runs it when the connection is found and read by NetworkManager.
This gets done by the system_handle_dcb_setting() function for any
connection that has a DCB setting, even if that interface is not
present?  Is that useful?  I'd actually expect that instead of sending
the DCB commands when the connection showed up, instead that would be
done when NetworkManager found a new network device, and if any
connections were compatible with that network device, then the DCB stuff
would get run from those compatible connections.
Yes. As for fcoe, it should wait until the interface is up, and for iscsi, it should wait until it has ip configuration.

(Also, what happens if there are more than one connection with DCB info
for the same device?  I think the patches apply both sets of info.)
Yes. I think one interface can support only one DCB setting.
I'm wondering if it really makes sense to store the info in ifcfg files
along with the interface's IP configuration and such, since it's really
independent of that.  At least on Fedora/RH, ifcfg files are a weird
conglomeration of both lower level stuff like MTU and duplex and higher
level stuff like IP addressing.  On the other hand, it would probably be
rare to physically move a machine to a different network and keep all
the DCB information the same, since it does seem quite network-specific.
Plus keeping the info together helps management since all the config is
in the same place (the connection).
We can split ifcfg file.
One crazy thought I had: what if the DCB stuff was more of a "plugin",
where NM called it at certain times for certain events (like
connection-added, connection-removed, interface-added,
interface-removed, interface-activated, etc) and the DCB plugin could
use the information passed along with the event (stuff like HW address,
interface name, connection details) to look up the relevant DCB
configuration information and send it to lldpad?  That way we could
achieve the NM event integration, but allow more flexibility with the
DCB configuration.
That is really a good suggestion.
Does "plugin" mean callback functions here ?

  That doesn't solve configuration management issues
though, since the configuration info wouldn't necessarily be in the same
place as the IP configuration info, for the reasons suggested below.


One thing I'm a bit worried about here is the proliferation of config
options.  You may have noticed that it's a bit complicated to add new
settings; that's something I'd like to change.  There's not a lot of
flexibility once the options are created, and the DCB patch adds a *lot*
of options.  The problem we might face is that perhaps lldpad releases a
new version that changes some config options, and now NetworkManager has
to work with both  old *and* new lldpad versions, because we keep the
same NM API on a stable branch, and we usually don't bump required
versions of major dependencies either.
Yes, if we want to use lldpad, we have to introduce this dependency.
How about just choosing the minimum DCB options that NM supports ?
We can add new options when it is really needed.

Also, are we expecting some sort of GUI editor for these options too, at
some future point?  Is that what users want, or are they satisfied with
editing config files directly for now?
At present, I think it is enough to let the users edit ifcfg file directly.

thanks
Weiping Pan
Dan

This patchset is on top of commit c3b29cec713f(ip6: only change ra_flags with
device_set_ra_flags()).

1 DCB ifcfg example

TYPE=Ethernet
DEVICE=eth0
HWADDR=00:11:22:33:44:ee
BOOTPROTO=none
IPADDR=1.2.3.4
PREFIX=24
GATEWAY=1.1.1.1

# dcb global switch
# use DCB=off to turn it off
DCB=on

# application settings
# It also supports DCB_APP_ISCSI_XXX and DCB_APP_FIP_XXX.
DCB_APP_FCOE=yes
DCB_APP_FCOE_CFG=08
DCB_APP_FCOE_ENABLE=yes
DCB_APP_FCOE_ADVERTISE=yes
DCB_APP_FCOE_WILLING=yes

DCB_APP_ISCSI=yes
DCB_APP_ISCSI_CFG=40
DCB_APP_ISCSI_ENABLE=yes
DCB_APP_ISCSI_ADVERTISE=yes
DCB_APP_ISCSI_WILLING=yes

# priority group settings
DCB_PG_UP2TC=76543210
DCB_PG_PCT=25,0,0,75,0,0,0,0
DCB_PG_ID=0000111f
DCB_PG_UPPCT=25,25,25,25,50,25,25,0
DCB_PG_STRICT=01010101
DCB_PG_ENABLE=yes
DCB_PG_ADVERTISE=yes
DCB_PG_WILLING=yes

# priority flow control settings
DCB_PFC_UP=00010000
DCB_PFC_ENABLE=yes
DCB_PFC_ADVERTISE=yes
DCB_PFC_WILLING=no

NOTE:
1 What forms a minimum valid ifcfg for DCB?
DCB=on

DCB_APP_FCOE=yes
DCB_APP_FCOE_ENABLE=yes
DCB_APP_FCOE_ADVERTISE=yes
DCB_APP_FCOE_WILLING=yes

DCB_PFC_ENABLE=yes
DCB_PFC_ADVERTISE=yes
DCB_PFC_WILLING=yes

2 For application settings, it also supports DCB_APP_ISCSI_XXX and
DCB_APP_FIP_XXX.

TEST:
I compared the commands (messages that liblldp_clif will send to lldpad) that
are generated by NetworkManager and dcbtool, and tried most combinations,
and can confirm that NetworkManager will generate the same commands with
dcbtool in the same situation.

TODO:
1 lldpad monitor
If lldpad daemon has not been started, NetworkManager should start it, and
postpone activation of DCB capable interface.
2 test it on real hardware
I will test the whole patchset with real hardware (FCoE and iSCSI).

Changelog:
V2: make it accept more than one application settings,
store app settings with GSList.
V3: add ifcfg-dcb writer


Weiping Pan (3):
   ifcfg-rh: add ifcfg-dcb parser
   DCB: generate commands and send them to lldpad
   ifcfg-rh: add ifcfg-dcb writer

  configure.ac                                       |   17 +
  docs/api/generate-settings-spec.c                  |    2 +
  docs/libnm-util/libnm-util-docs.sgml               |    1 +
  include/nm-dbus-glib-types.h                       |    2 +
  libnm-util/Makefile.am                             |    2 +
  libnm-util/libnm-util.ver                          |   26 +
  libnm-util/nm-connection.c                         |   24 +-
  libnm-util/nm-connection.h                         |    2 +
  libnm-util/nm-setting-dcb.c                        | 1185 ++++++++++++++++++++
  libnm-util/nm-setting-dcb.h                        |  165 +++
  libnm-util/nm-utils.c                              |  191 ++++
  libnm-util/nm-utils.h                              |   10 +
  src/Makefile.am                                    |    1 +
  src/nm-manager.c                                   |   94 ++
  src/settings/plugins/ifcfg-rh/reader.c             |  479 ++++++++-
  .../ifcfg-rh/tests/network-scripts/ifcfg-test-dcb  |   47 +
  .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c         |   84 ++
  src/settings/plugins/ifcfg-rh/writer.c             |  189 ++++
  18 files changed, 2519 insertions(+), 2 deletions(-)
  create mode 100644 libnm-util/nm-setting-dcb.c
  create mode 100644 libnm-util/nm-setting-dcb.h
  create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dcb





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]