Re: [PATCH net-next] net/ncsi: Add generic netlink family

[Samuel Mendoza-Jonas]

On Mon, 2018-02-26 at 11:31 -0500, David Miller wrote:
> From: Samuel Mendoza-Jonas 
> Date: Fri, 23 Feb 2018 15:15:18 +1100
> 
> > + * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
> > + *   Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
> > + *   optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
> > + *   specified the setting is cleared.
> 
> I think clearing the setting when the required attributes are missing
> is dangerous behavior.
> 
> It is ambiguous whether the user intended the setting to be cleared,
> or was in error and forgot to supply the attribute due to a bug.

Fair point - I'll change this to be an error and add a separate command
to clear the setting explicitly.
In that vein is having NCSI_ATTR_CHANNEL_ID as an optional parameter
ambiguous enough to justify a separate command as well?

Regards,
Sam

Re: [PATCH net-next] net/ncsi: Add generic netlink family

[Samuel Mendoza-Jonas]

On Mon, 2018-02-26 at 11:31 -0500, David Miller wrote:
> From: Samuel Mendoza-Jonas 
> Date: Fri, 23 Feb 2018 15:15:18 +1100
> 
> > + * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
> > + *   Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
> > + *   optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
> > + *   specified the setting is cleared.
> 
> I think clearing the setting when the required attributes are missing
> is dangerous behavior.
> 
> It is ambiguous whether the user intended the setting to be cleared,
> or was in error and forgot to supply the attribute due to a bug.

Fair point - I'll change this to be an error and add a separate command
to clear the setting explicitly.
In that vein is having NCSI_ATTR_CHANNEL_ID as an optional parameter
ambiguous enough to justify a separate command as well?

Regards,
Sam

Re: [PATCH net-next] net/ncsi: Add generic netlink family

[David Miller]

From: Samuel Mendoza-Jonas 
Date: Fri, 23 Feb 2018 15:15:18 +1100

> + * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
> + *   Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
> + *   optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
> + *   specified the setting is cleared.

I think clearing the setting when the required attributes are missing
is dangerous behavior.

It is ambiguous whether the user intended the setting to be cleared,
or was in error and forgot to supply the attribute due to a bug.

Re: [PATCH net-next] net/ncsi: Add generic netlink family

[David Miller]

From: Samuel Mendoza-Jonas 
Date: Fri, 23 Feb 2018 15:15:18 +1100

> + * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
> + *   Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
> + *   optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
> + *   specified the setting is cleared.

I think clearing the setting when the required attributes are missing
is dangerous behavior.

It is ambiguous whether the user intended the setting to be cleared,
or was in error and forgot to supply the attribute due to a bug.

[PATCH net-next] net/ncsi: Add generic netlink family

[Samuel Mendoza-Jonas]

Add a generic netlink family for NCSI. This supports two commands;
NCSI_CMD_PKG_INFO which returns information on packages and their
associated channels, and NCSI_CMD_SET_INTERFACE which allows a specific
package or package/channel combination to be set as the preferred
choice.

Signed-off-by: Samuel Mendoza-Jonas 
---
 include/uapi/linux/ncsi.h | 113 +
 net/ncsi/Makefile |   2 +-
 net/ncsi/internal.h   |   3 +
 net/ncsi/ncsi-manage.c|  30 +++-
 net/ncsi/ncsi-netlink.c   | 394 ++
 net/ncsi/ncsi-netlink.h   |  20 +++
 6 files changed, 557 insertions(+), 5 deletions(-)
 create mode 100644 include/uapi/linux/ncsi.h
 create mode 100644 net/ncsi/ncsi-netlink.c
 create mode 100644 net/ncsi/ncsi-netlink.h

diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
new file mode 100644
index ..aecab3fb92df
--- /dev/null
+++ b/include/uapi/linux/ncsi.h
@@ -0,0 +1,113 @@
+/*
+ * Copyright Samuel Mendoza-Jonas, IBM Corporation 2018.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __UAPI_NCSI_NETLINK_H__
+#define __UAPI_NCSI_NETLINK_H__
+
+/**
+ * enum ncsi_nl_commands - supported NCSI commands
+ *
+ * @NCSI_CMD_UNSPEC: unspecified command to catch errors
+ * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
+ * Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
+ * optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
+ * specified the setting is cleared.
+ * @NCSI_CMD_PKG_INFO: list package and channel attributes. Requires
+ * NCSI_ATTR_IFINDEX. If NCSI_ATTR_PACKAGE_ID is specified returns the
+ * specific package and its channels - otherwise a dump request returns
+ * all packages and their associated channels.
+ * @NCSI_CMD_MAX: highest command number
+ */
+enum ncsi_nl_commands {
+   NCSI_CMD_UNSPEC,
+   NCSI_CMD_SET_INTERFACE,
+   NCSI_CMD_PKG_INFO,
+
+   __NCSI_CMD_AFTER_LAST,
+   NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_attrs - General NCSI netlink attributes
+ *
+ * @NCSI_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_ATTR_IFINDEX: ifindex of network device using NCSI
+ * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
+ * @NCSI_ATTR_PACKAGE_ID: package ID
+ * @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_attrs {
+   NCSI_ATTR_UNSPEC,
+   NCSI_ATTR_IFINDEX,
+   NCSI_ATTR_PACKAGE_LIST,
+   NCSI_ATTR_PACKAGE_ID,
+   NCSI_ATTR_CHANNEL_ID,
+
+   __NCSI_ATTR_AFTER_LAST,
+   NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_pkg_attrs - NCSI netlink package-specific attributes
+ *
+ * @NCSI_PKG_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_PKG_ATTR: nested array of package attributes
+ * @NCSI_PKG_ATTR_ID: package ID
+ * @NCSI_PKG_ATTR_FORCED: flag signifying a package has been set as preferred
+ * @NCSI_PKG_ATTR_CHANNEL_LIST: nested array of NCSI_CHANNEL_ATTR attributes
+ * @NCSI_PKG_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_pkg_attrs {
+   NCSI_PKG_ATTR_UNSPEC,
+   NCSI_PKG_ATTR,
+   NCSI_PKG_ATTR_ID,
+   NCSI_PKG_ATTR_FORCED,
+   NCSI_PKG_ATTR_CHANNEL_LIST,
+
+   __NCSI_PKG_ATTR_AFTER_LAST,
+   NCSI_PKG_ATTR_MAX = __NCSI_PKG_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_channel_attrs - NCSI netlink channel-specific attributes
+ *
+ * @NCSI_CHANNEL_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_CHANNEL_ATTR: nested array of channel attributes
+ * @NCSI_CHANNEL_ATTR_ID: channel ID
+ * @NCSI_CHANNEL_ATTR_VERSION_MAJOR: channel major version number
+ * @NCSI_CHANNEL_ATTR_VERSION_MINOR: channel minor version number
+ * @NCSI_CHANNEL_ATTR_VERSION_STR: channel version string
+ * @NCSI_CHANNEL_ATTR_LINK_STATE: channel link state flags
+ * @NCSI_CHANNEL_ATTR_ACTIVE: channels with this flag are in
+ * NCSI_CHANNEL_ACTIVE state
+ * @NCSI_CHANNEL_ATTR_FORCED: flag signifying a channel has been set as
+ * preferred
+ * @NCSI_CHANNEL_ATTR_VLAN_LIST: nested array of NCSI_CHANNEL_ATTR_VLAN_IDs
+ * @NCSI_CHANNEL_ATTR_VLAN_ID: VLAN ID being filtered on this channel
+ * @NCSI_CHANNEL_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_channel_attrs {
+   NCSI_CHANNEL_ATTR_UNSPEC,
+   NCSI_CHANNEL_ATTR,
+   NCSI_CHANNEL_ATTR_ID,
+   NCSI_CHANNEL_ATTR_VERSION_MAJOR,
+   NCSI_CHANNEL_ATTR_VERSION_MINOR,
+   NCSI_CHANNEL_ATTR_VERSION_STR,
+   NCSI_CHANNEL_ATTR_LINK_STATE,
+   NCSI_CHANNEL_ATTR_ACTIVE,
+   NCSI_CHANNEL_ATTR_FORCED,
+   NCSI_CHANNEL_ATTR_VLAN_LIST,
+   NCSI_CHANNEL_ATTR_VLAN_ID,
+
+   

[PATCH net-next] net/ncsi: Add generic netlink family

[Samuel Mendoza-Jonas]

Add a generic netlink family for NCSI. This supports two commands;
NCSI_CMD_PKG_INFO which returns information on packages and their
associated channels, and NCSI_CMD_SET_INTERFACE which allows a specific
package or package/channel combination to be set as the preferred
choice.

Signed-off-by: Samuel Mendoza-Jonas 
---
 include/uapi/linux/ncsi.h | 113 +
 net/ncsi/Makefile |   2 +-
 net/ncsi/internal.h   |   3 +
 net/ncsi/ncsi-manage.c|  30 +++-
 net/ncsi/ncsi-netlink.c   | 394 ++
 net/ncsi/ncsi-netlink.h   |  20 +++
 6 files changed, 557 insertions(+), 5 deletions(-)
 create mode 100644 include/uapi/linux/ncsi.h
 create mode 100644 net/ncsi/ncsi-netlink.c
 create mode 100644 net/ncsi/ncsi-netlink.h

diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
new file mode 100644
index ..aecab3fb92df
--- /dev/null
+++ b/include/uapi/linux/ncsi.h
@@ -0,0 +1,113 @@
+/*
+ * Copyright Samuel Mendoza-Jonas, IBM Corporation 2018.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __UAPI_NCSI_NETLINK_H__
+#define __UAPI_NCSI_NETLINK_H__
+
+/**
+ * enum ncsi_nl_commands - supported NCSI commands
+ *
+ * @NCSI_CMD_UNSPEC: unspecified command to catch errors
+ * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
+ * Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
+ * optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
+ * specified the setting is cleared.
+ * @NCSI_CMD_PKG_INFO: list package and channel attributes. Requires
+ * NCSI_ATTR_IFINDEX. If NCSI_ATTR_PACKAGE_ID is specified returns the
+ * specific package and its channels - otherwise a dump request returns
+ * all packages and their associated channels.
+ * @NCSI_CMD_MAX: highest command number
+ */
+enum ncsi_nl_commands {
+   NCSI_CMD_UNSPEC,
+   NCSI_CMD_SET_INTERFACE,
+   NCSI_CMD_PKG_INFO,
+
+   __NCSI_CMD_AFTER_LAST,
+   NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_attrs - General NCSI netlink attributes
+ *
+ * @NCSI_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_ATTR_IFINDEX: ifindex of network device using NCSI
+ * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
+ * @NCSI_ATTR_PACKAGE_ID: package ID
+ * @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_attrs {
+   NCSI_ATTR_UNSPEC,
+   NCSI_ATTR_IFINDEX,
+   NCSI_ATTR_PACKAGE_LIST,
+   NCSI_ATTR_PACKAGE_ID,
+   NCSI_ATTR_CHANNEL_ID,
+
+   __NCSI_ATTR_AFTER_LAST,
+   NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_pkg_attrs - NCSI netlink package-specific attributes
+ *
+ * @NCSI_PKG_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_PKG_ATTR: nested array of package attributes
+ * @NCSI_PKG_ATTR_ID: package ID
+ * @NCSI_PKG_ATTR_FORCED: flag signifying a package has been set as preferred
+ * @NCSI_PKG_ATTR_CHANNEL_LIST: nested array of NCSI_CHANNEL_ATTR attributes
+ * @NCSI_PKG_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_pkg_attrs {
+   NCSI_PKG_ATTR_UNSPEC,
+   NCSI_PKG_ATTR,
+   NCSI_PKG_ATTR_ID,
+   NCSI_PKG_ATTR_FORCED,
+   NCSI_PKG_ATTR_CHANNEL_LIST,
+
+   __NCSI_PKG_ATTR_AFTER_LAST,
+   NCSI_PKG_ATTR_MAX = __NCSI_PKG_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_channel_attrs - NCSI netlink channel-specific attributes
+ *
+ * @NCSI_CHANNEL_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_CHANNEL_ATTR: nested array of channel attributes
+ * @NCSI_CHANNEL_ATTR_ID: channel ID
+ * @NCSI_CHANNEL_ATTR_VERSION_MAJOR: channel major version number
+ * @NCSI_CHANNEL_ATTR_VERSION_MINOR: channel minor version number
+ * @NCSI_CHANNEL_ATTR_VERSION_STR: channel version string
+ * @NCSI_CHANNEL_ATTR_LINK_STATE: channel link state flags
+ * @NCSI_CHANNEL_ATTR_ACTIVE: channels with this flag are in
+ * NCSI_CHANNEL_ACTIVE state
+ * @NCSI_CHANNEL_ATTR_FORCED: flag signifying a channel has been set as
+ * preferred
+ * @NCSI_CHANNEL_ATTR_VLAN_LIST: nested array of NCSI_CHANNEL_ATTR_VLAN_IDs
+ * @NCSI_CHANNEL_ATTR_VLAN_ID: VLAN ID being filtered on this channel
+ * @NCSI_CHANNEL_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_channel_attrs {
+   NCSI_CHANNEL_ATTR_UNSPEC,
+   NCSI_CHANNEL_ATTR,
+   NCSI_CHANNEL_ATTR_ID,
+   NCSI_CHANNEL_ATTR_VERSION_MAJOR,
+   NCSI_CHANNEL_ATTR_VERSION_MINOR,
+   NCSI_CHANNEL_ATTR_VERSION_STR,
+   NCSI_CHANNEL_ATTR_LINK_STATE,
+   NCSI_CHANNEL_ATTR_ACTIVE,
+   NCSI_CHANNEL_ATTR_FORCED,
+   NCSI_CHANNEL_ATTR_VLAN_LIST,
+   NCSI_CHANNEL_ATTR_VLAN_ID,
+
+   __NCSI_CHANNEL_ATTR_AFTER_LAST,
+