* [PATCH 0/2] Display adjacent switch port's attributes
@ 2013-12-11 13:28 Stefan Raspl
2013-12-11 13:28 ` [PATCH 1/2] ethtool: Add callback to indicate adjacent switch port attributes Stefan Raspl
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Stefan Raspl @ 2013-12-11 13:28 UTC (permalink / raw)
To: davem; +Cc: bhutchings, blaschka, netdev, linux-s390, Stefan Raspl
This patch series adds a new callback for ethtool to display the adjacent switch
port's attributes, as perceived by the NIC, e.g. through respective LLDP message
exchanges.
A patch for the qeth device driver provides a sample exploiter for the new
functionality.
A new option for ethtool to display the settings will be posted in a separate
patch.
Best regards,
Stefan Raspl
Stefan Raspl (2):
ethtool: Add callback to indicate adjacent switch port attributes
qeth: Display adjacent switch port attributes in ethtool
drivers/s390/net/qeth_core.h | 7 ++++
drivers/s390/net/qeth_core_main.c | 79 +++++++++++++++++++++++++++++++++++++++
drivers/s390/net/qeth_core_mpc.h | 17 +++++++++
drivers/s390/net/qeth_l2_main.c | 1 +
drivers/s390/net/qeth_l3_main.c | 1 +
include/linux/ethtool.h | 3 ++
include/uapi/linux/ethtool.h | 35 +++++++++++++++++
net/core/ethtool.c | 22 +++++++++++
8 files changed, 165 insertions(+)
--
1.8.3.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] ethtool: Add callback to indicate adjacent switch port attributes 2013-12-11 13:28 [PATCH 0/2] Display adjacent switch port's attributes Stefan Raspl @ 2013-12-11 13:28 ` Stefan Raspl 2013-12-11 13:29 ` [PATCH 2/2] qeth: Display adjacent switch port attributes in ethtool Stefan Raspl 2013-12-11 20:13 ` [PATCH 0/2] Display adjacent switch port's attributes Stephen Hemminger 2 siblings, 0 replies; 15+ messages in thread From: Stefan Raspl @ 2013-12-11 13:28 UTC (permalink / raw) To: davem; +Cc: bhutchings, blaschka, netdev, linux-s390, Stefan Raspl Switches supporting LLDP can communicate port attributes to connected devices. Device drivers capable of accessing this information from the devices can use the new callback get_switch_port_attrs() to report supported and enabled settings in the card's adjacent switch port for display in ethtool. Implementors have to use the respective SUPPORTED_SP_* and ENABLED_SP_* defines to indicate the current settings. Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com> --- include/linux/ethtool.h | 3 +++ include/uapi/linux/ethtool.h | 35 +++++++++++++++++++++++++++++++++++ net/core/ethtool.c | 22 ++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index c8e3e7e3..940c7b1 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -177,6 +177,7 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings) * @get_module_eeprom: Get the eeprom information from the plug-in module * @get_eee: Get Energy-Efficient (EEE) supported and status. * @set_eee: Set EEE status (enable/disable) as well as LPI timers. + * @get_switch_port_attrs: Get adjacent switch port attributes. * * All operations are optional (i.e. the function pointer may be set * to %NULL) and callers must take this into account. Callers must @@ -245,6 +246,8 @@ struct ethtool_ops { struct ethtool_eeprom *, u8 *); int (*get_eee)(struct net_device *, struct ethtool_eee *); int (*set_eee)(struct net_device *, struct ethtool_eee *); + int (*get_switch_port_attrs)(struct net_device *, + struct ethtool_swport_attrs *); }; diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index 38dbafa..f5843ac 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -136,6 +136,22 @@ struct ethtool_eeprom { }; /** + * struct ethtool_swport_attrs - query adjacent switch port attributes + * @cmd: ETHTOOL_GPORT + * @port_rc: Use GPORT_RC_* as appropriate. + * @supported: Forwarding modes and capabilities supported by the switch port, + * see SUPPORTED_SP_* flags. + * @enabled: Forwarding modes and capabilities currently activated at the + * adjacent switch port, see ENABLED_SP_* flags. + */ +struct ethtool_swport_attrs { + __u32 cmd; + __u32 port_rc; + __u32 supported; + __u32 enabled; +}; + +/** * struct ethtool_eee - Energy Efficient Ethernet information * @cmd: ETHTOOL_{G,S}EEE * @supported: Mask of %SUPPORTED_* flags for the speed/duplex combinations @@ -900,6 +916,7 @@ enum ethtool_sfeatures_retval_bits { #define ETHTOOL_GMODULEEEPROM 0x00000043 /* Get plug-in module eeprom */ #define ETHTOOL_GEEE 0x00000044 /* Get EEE settings */ #define ETHTOOL_SEEE 0x00000045 /* Set EEE settings */ +#define ETHTOOL_GPORT 0x00000046 /* Get switch port attributes */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET @@ -1067,6 +1084,24 @@ enum ethtool_sfeatures_retval_bits { #define ETH_MODULE_SFF_8472 0x2 #define ETH_MODULE_SFF_8472_LEN 512 +/* Bad return codes for switch ports */ +#define GPORT_RC_LLDP_UNSUP 1 /* switch port doesn't support */ + /* required LLDP EVB TLV */ + +/* Indicates what features the adjacent switch port supports. */ +#define SUPPORTED_SP_FWD_802_1 (1 << 0) +#define SUPPORTED_SP_FWD_RR (1 << 1) +#define SUPPORTED_SP_CAP_RTE (1 << 9) +#define SUPPORTED_SP_CAP_ECP (1 << 10) +#define SUPPORTED_SP_CAP_VDP (1 << 11) + +/* Indicates what features the adjacent switch port has enabled. */ +#define ENABLED_SP_FWD_802_1 (1 << 0) +#define ENABLED_SP_FWD_RR (1 << 1) +#define ENABLED_SP_CAP_RTE (1 << 9) +#define ENABLED_SP_CAP_ECP (1 << 10) +#define ENABLED_SP_CAP_VDP (1 << 11) + /* Reset flags */ /* The reset() operation must clear the flags for the components which * were actually reset. On successful return, the flags indicate the diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 30071de..84f69f1 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -1446,6 +1446,25 @@ static int ethtool_get_module_eeprom(struct net_device *dev, modinfo.eeprom_len); } +static int ethtool_get_switch_port_attrs(struct net_device *dev, + void __user *useraddr) +{ + struct ethtool_swport_attrs attrs = { ETHTOOL_GPORT }; + int rc; + + if (!dev->ethtool_ops->get_switch_port_attrs) + return -EOPNOTSUPP; + + rc = dev->ethtool_ops->get_switch_port_attrs(dev, &attrs); + if (rc) + return rc; + + if (copy_to_user(useraddr, &attrs, sizeof(attrs))) + return -EFAULT; + + return 0; +} + /* The main entry point in this file. Called from net/core/dev_ioctl.c */ int dev_ethtool(struct net *net, struct ifreq *ifr) @@ -1675,6 +1694,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) case ETHTOOL_GMODULEEEPROM: rc = ethtool_get_module_eeprom(dev, useraddr); break; + case ETHTOOL_GPORT: + rc = ethtool_get_switch_port_attrs(dev, useraddr); + break; default: rc = -EOPNOTSUPP; } -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] qeth: Display adjacent switch port attributes in ethtool 2013-12-11 13:28 [PATCH 0/2] Display adjacent switch port's attributes Stefan Raspl 2013-12-11 13:28 ` [PATCH 1/2] ethtool: Add callback to indicate adjacent switch port attributes Stefan Raspl @ 2013-12-11 13:29 ` Stefan Raspl 2013-12-11 20:13 ` [PATCH 0/2] Display adjacent switch port's attributes Stephen Hemminger 2 siblings, 0 replies; 15+ messages in thread From: Stefan Raspl @ 2013-12-11 13:29 UTC (permalink / raw) To: davem; +Cc: bhutchings, blaschka, netdev, linux-s390, Stefan Raspl Add support for get_switch_port_attrs() callback in ethtool to display adjacent switch port's attributes. Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com> Acked-by: Frank Blaschka <blaschka@linux.vnet.ibm.com> --- drivers/s390/net/qeth_core.h | 7 ++++ drivers/s390/net/qeth_core_main.c | 79 +++++++++++++++++++++++++++++++++++++++ drivers/s390/net/qeth_core_mpc.h | 17 +++++++++ drivers/s390/net/qeth_l2_main.c | 1 + drivers/s390/net/qeth_l3_main.c | 1 + 5 files changed, 105 insertions(+) diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h index 41ef943..bc30a2a 100644 --- a/drivers/s390/net/qeth_core.h +++ b/drivers/s390/net/qeth_core.h @@ -738,6 +738,11 @@ struct qeth_rx { int qdio_err; }; +struct qeth_switch_info { + __u32 capabilities; + __u32 settings; +}; + #define QETH_NAPI_WEIGHT NAPI_POLL_WEIGHT struct qeth_card { @@ -914,6 +919,8 @@ struct qeth_cmd_buffer *qeth_wait_for_buffer(struct qeth_channel *); int qeth_mdio_read(struct net_device *, int, int); int qeth_snmp_command(struct qeth_card *, char __user *); int qeth_query_oat_command(struct qeth_card *, char __user *); +int qeth_core_ethtool_get_switch_port_attrs(struct net_device *, + struct ethtool_swport_attrs *); int qeth_send_control_data(struct qeth_card *, int, struct qeth_cmd_buffer *, int (*reply_cb)(struct qeth_card *, struct qeth_reply*, unsigned long), void *reply_param); diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c index eb4e1f8..b8827bd 100644 --- a/drivers/s390/net/qeth_core_main.c +++ b/drivers/s390/net/qeth_core_main.c @@ -3036,6 +3036,42 @@ int qeth_query_ipassists(struct qeth_card *card, enum qeth_prot_versions prot) } EXPORT_SYMBOL_GPL(qeth_query_ipassists); +static int qeth_query_switch_port_attributes_cb(struct qeth_card *card, + struct qeth_reply *reply, unsigned long data) +{ + struct qeth_ipa_cmd *cmd; + struct qeth_switch_info *sw_info; + struct qeth_query_switch_attributes *attrs; + + QETH_CARD_TEXT(card, 2, "qsqiatcb"); + cmd = (struct qeth_ipa_cmd *) data; + sw_info = (struct qeth_switch_info *)reply->param; + if (cmd->data.setadapterparms.hdr.return_code == 0) { + attrs = &cmd->data.setadapterparms.data.query_switch_attributes; + sw_info->capabilities = attrs->capabilities; + sw_info->settings = attrs->settings; + } + qeth_default_setadapterparms_cb(card, reply, (unsigned long) cmd); + + return 0; +} + +static int qeth_query_switch_port_attributes(struct qeth_card *card, + struct qeth_switch_info *sw_info) +{ + struct qeth_cmd_buffer *iob; + + QETH_CARD_TEXT(card, 2, "qswiattr"); + if (!qeth_adp_supported(card, IPA_SETADP_QUERY_SWITCH_ATTRIBUTES)) + return -EOPNOTSUPP; + if (!netif_carrier_ok(card->dev)) + return -ENOMEDIUM; + iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_SWITCH_ATTRIBUTES, + sizeof(struct qeth_ipacmd_setadpparms_hdr)); + return qeth_send_ipa_cmd(card, iob, + qeth_query_switch_port_attributes_cb, sw_info); +} + static int qeth_query_setdiagass_cb(struct qeth_card *card, struct qeth_reply *reply, unsigned long data) { @@ -5696,6 +5732,49 @@ int qeth_core_ethtool_get_settings(struct net_device *netdev, } EXPORT_SYMBOL_GPL(qeth_core_ethtool_get_settings); +int qeth_core_ethtool_get_switch_port_attrs(struct net_device *netdev, + struct ethtool_swport_attrs *attrs) +{ + int rc, i; + struct qeth_card *card = netdev->ml_priv; + struct qeth_switch_info sw_info; + static const struct { + int qeth_cap; + int etht_cap_sup; + int etht_cap_enbl; + } qattrs[] = { + { QETH_SWITCH_FORW_802_1, SUPPORTED_SP_FWD_802_1, + ENABLED_SP_FWD_802_1 }, + { QETH_SWITCH_FORW_REFL_RELAY, SUPPORTED_SP_FWD_RR, + ENABLED_SP_FWD_RR }, + { QETH_SWITCH_CAP_RTE, SUPPORTED_SP_CAP_RTE, + ENABLED_SP_CAP_RTE }, + { QETH_SWITCH_CAP_ECP, SUPPORTED_SP_CAP_ECP, + ENABLED_SP_CAP_ECP }, + { QETH_SWITCH_CAP_VDP, SUPPORTED_SP_CAP_VDP, + ENABLED_SP_CAP_VDP } + }; + + rc = qeth_query_switch_port_attributes(card, &sw_info); + if (rc) + return rc; + + if (!sw_info.capabilities) { + attrs->port_rc = GPORT_RC_LLDP_UNSUP; + return 0; + } + for (i = 0; i < ARRAY_SIZE(qattrs); i++) { + if (sw_info.capabilities & qattrs[i].qeth_cap) { + attrs->supported |= qattrs[i].etht_cap_sup; + if (sw_info.settings & qattrs[i].qeth_cap) + attrs->enabled |= qattrs[i].etht_cap_enbl; + } + } + + return 0; +} +EXPORT_SYMBOL_GPL(qeth_core_ethtool_get_switch_port_attrs); + static int __init qeth_core_init(void) { int rc; diff --git a/drivers/s390/net/qeth_core_mpc.h b/drivers/s390/net/qeth_core_mpc.h index 07085d5..9f9b034 100644 --- a/drivers/s390/net/qeth_core_mpc.h +++ b/drivers/s390/net/qeth_core_mpc.h @@ -240,6 +240,7 @@ enum qeth_ipa_setadp_cmd { IPA_SETADP_SET_DIAG_ASSIST = 0x00002000L, IPA_SETADP_SET_ACCESS_CONTROL = 0x00010000L, IPA_SETADP_QUERY_OAT = 0x00080000L, + IPA_SETADP_QUERY_SWITCH_ATTRIBUTES = 0x00100000L, }; enum qeth_ipa_mac_ops { CHANGE_ADDR_READ_MAC = 0, @@ -404,6 +405,21 @@ struct qeth_qoat_priv { char *buffer; }; +#define QETH_SWITCH_FORW_802_1 0x00000001 +#define QETH_SWITCH_FORW_REFL_RELAY 0x00000002 +#define QETH_SWITCH_CAP_RTE 0x00000004 +#define QETH_SWITCH_CAP_ECP 0x00000008 +#define QETH_SWITCH_CAP_VDP 0x00000010 + +struct qeth_query_switch_attributes { + __u8 version; + __u8 reserved1; + __u16 reserved2; + __u32 capabilities; + __u32 settings; + __u8 reserved3[8]; +}; + struct qeth_ipacmd_setadpparms_hdr { __u32 supp_hw_cmds; __u32 reserved1; @@ -424,6 +440,7 @@ struct qeth_ipacmd_setadpparms { struct qeth_snmp_cmd snmp; struct qeth_set_access_ctrl set_access_ctrl; struct qeth_query_oat query_oat; + struct qeth_query_switch_attributes query_switch_attributes; __u32 mode; } data; } __attribute__ ((packed)); diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c index ec8ccda..ae4de33 100644 --- a/drivers/s390/net/qeth_l2_main.c +++ b/drivers/s390/net/qeth_l2_main.c @@ -911,6 +911,7 @@ static const struct ethtool_ops qeth_l2_ethtool_ops = { .get_sset_count = qeth_core_get_sset_count, .get_drvinfo = qeth_core_get_drvinfo, .get_settings = qeth_core_ethtool_get_settings, + .get_switch_port_attrs = qeth_core_ethtool_get_switch_port_attrs, }; static const struct ethtool_ops qeth_l2_osn_ops = { diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c index c1b0b27..356076a 100644 --- a/drivers/s390/net/qeth_l3_main.c +++ b/drivers/s390/net/qeth_l3_main.c @@ -3198,6 +3198,7 @@ static const struct ethtool_ops qeth_l3_ethtool_ops = { .get_sset_count = qeth_core_get_sset_count, .get_drvinfo = qeth_core_get_drvinfo, .get_settings = qeth_core_ethtool_get_settings, + .get_switch_port_attrs = qeth_core_ethtool_get_switch_port_attrs, }; /* -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-11 13:28 [PATCH 0/2] Display adjacent switch port's attributes Stefan Raspl 2013-12-11 13:28 ` [PATCH 1/2] ethtool: Add callback to indicate adjacent switch port attributes Stefan Raspl 2013-12-11 13:29 ` [PATCH 2/2] qeth: Display adjacent switch port attributes in ethtool Stefan Raspl @ 2013-12-11 20:13 ` Stephen Hemminger 2013-12-12 10:06 ` Nicolas Dichtel 2013-12-12 13:57 ` Stefan Raspl 2 siblings, 2 replies; 15+ messages in thread From: Stephen Hemminger @ 2013-12-11 20:13 UTC (permalink / raw) To: Stefan Raspl; +Cc: davem, bhutchings, blaschka, netdev, linux-s390 On Wed, 11 Dec 2013 14:28:58 +0100 Stefan Raspl <raspl@linux.vnet.ibm.com> wrote: > This patch series adds a new callback for ethtool to display the adjacent switch > port's attributes, as perceived by the NIC, e.g. through respective LLDP message > exchanges. > A patch for the qeth device driver provides a sample exploiter for the new > functionality. > A new option for ethtool to display the settings will be posted in a separate > patch. > > Best regards, > Stefan Raspl > > Stefan Raspl (2): > ethtool: Add callback to indicate adjacent switch port attributes > qeth: Display adjacent switch port attributes in ethtool > > drivers/s390/net/qeth_core.h | 7 ++++ > drivers/s390/net/qeth_core_main.c | 79 +++++++++++++++++++++++++++++++++++++++ > drivers/s390/net/qeth_core_mpc.h | 17 +++++++++ > drivers/s390/net/qeth_l2_main.c | 1 + > drivers/s390/net/qeth_l3_main.c | 1 + > include/linux/ethtool.h | 3 ++ > include/uapi/linux/ethtool.h | 35 +++++++++++++++++ > net/core/ethtool.c | 22 +++++++++++ > 8 files changed, 165 insertions(+) > Why more ethtool and not netlink? ethtool is a brittle interface, non-extensible and does not support notifications of changes. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-11 20:13 ` [PATCH 0/2] Display adjacent switch port's attributes Stephen Hemminger @ 2013-12-12 10:06 ` Nicolas Dichtel 2013-12-12 13:57 ` Stefan Raspl 1 sibling, 0 replies; 15+ messages in thread From: Nicolas Dichtel @ 2013-12-12 10:06 UTC (permalink / raw) To: Stephen Hemminger, Stefan Raspl Cc: davem, bhutchings, blaschka, netdev, linux-s390 Le 11/12/2013 21:13, Stephen Hemminger a écrit : > On Wed, 11 Dec 2013 14:28:58 +0100 > Stefan Raspl <raspl@linux.vnet.ibm.com> wrote: > >> This patch series adds a new callback for ethtool to display the adjacent switch >> port's attributes, as perceived by the NIC, e.g. through respective LLDP message >> exchanges. >> A patch for the qeth device driver provides a sample exploiter for the new >> functionality. >> A new option for ethtool to display the settings will be posted in a separate >> patch. >> >> Best regards, >> Stefan Raspl >> >> Stefan Raspl (2): >> ethtool: Add callback to indicate adjacent switch port attributes >> qeth: Display adjacent switch port attributes in ethtool >> >> drivers/s390/net/qeth_core.h | 7 ++++ >> drivers/s390/net/qeth_core_main.c | 79 +++++++++++++++++++++++++++++++++++++++ >> drivers/s390/net/qeth_core_mpc.h | 17 +++++++++ >> drivers/s390/net/qeth_l2_main.c | 1 + >> drivers/s390/net/qeth_l3_main.c | 1 + >> include/linux/ethtool.h | 3 ++ >> include/uapi/linux/ethtool.h | 35 +++++++++++++++++ >> net/core/ethtool.c | 22 +++++++++++ >> 8 files changed, 165 insertions(+) >> > > Why more ethtool and not netlink? ethtool is a brittle interface, non-extensible > and does not support notifications of changes. I agree with Stephen, netlink has more pros. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-11 20:13 ` [PATCH 0/2] Display adjacent switch port's attributes Stephen Hemminger 2013-12-12 10:06 ` Nicolas Dichtel @ 2013-12-12 13:57 ` Stefan Raspl 2013-12-12 17:28 ` David Miller 1 sibling, 1 reply; 15+ messages in thread From: Stefan Raspl @ 2013-12-12 13:57 UTC (permalink / raw) To: Stephen Hemminger; +Cc: davem, bhutchings, blaschka, netdev, linux-s390 Am 11.12.2013 21:13, schrieb Stephen Hemminger: > On Wed, 11 Dec 2013 14:28:58 +0100 > Stefan Raspl <raspl@linux.vnet.ibm.com> wrote: > >> This patch series adds a new callback for ethtool to display the adjacent switch >> port's attributes, as perceived by the NIC, e.g. through respective LLDP message >> exchanges. >> A patch for the qeth device driver provides a sample exploiter for the new >> functionality. >> A new option for ethtool to display the settings will be posted in a separate >> patch. >> >> Best regards, >> Stefan Raspl >> >> Stefan Raspl (2): >> ethtool: Add callback to indicate adjacent switch port attributes >> qeth: Display adjacent switch port attributes in ethtool >> >> drivers/s390/net/qeth_core.h | 7 ++++ >> drivers/s390/net/qeth_core_main.c | 79 +++++++++++++++++++++++++++++++++++++++ >> drivers/s390/net/qeth_core_mpc.h | 17 +++++++++ >> drivers/s390/net/qeth_l2_main.c | 1 + >> drivers/s390/net/qeth_l3_main.c | 1 + >> include/linux/ethtool.h | 3 ++ >> include/uapi/linux/ethtool.h | 35 +++++++++++++++++ >> net/core/ethtool.c | 22 +++++++++++ >> 8 files changed, 165 insertions(+) >> > > Why more ethtool and not netlink? ethtool is a brittle interface, non-extensible > and does not support notifications of changes. > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html I agree, netlink would certainly be nice. But ethtool is the de-facto standard for now, and there doesn't seem to be a netlink-based alternative in sight - or is there? Offering a netlink based tool seems to be a different discussion, and if that takes shape, existing ethtool functionality (including this series) can and should be migrated. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 13:57 ` Stefan Raspl @ 2013-12-12 17:28 ` David Miller 2013-12-12 17:52 ` John Fastabend 0 siblings, 1 reply; 15+ messages in thread From: David Miller @ 2013-12-12 17:28 UTC (permalink / raw) To: raspl; +Cc: stephen, bhutchings, blaschka, netdev, linux-s390 From: Stefan Raspl <raspl@linux.vnet.ibm.com> Date: Thu, 12 Dec 2013 14:57:18 +0100 > I agree, netlink would certainly be nice. But ethtool is the > de-facto standard for now, and there doesn't seem to be a > netlink-based alternative in sight - or is there? > Offering a netlink based tool seems to be a different discussion, > and if that takes shape, existing ethtool functionality (including > this series) can and should be migrated. I completely disagree, ethtool is not de-facto for anything in particular. It's suitable for some things, not suitable for others. And just because it's used for other aspects of a device's configuration doesn't mean that netlink isn't appropriate for other apsects. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 17:28 ` David Miller @ 2013-12-12 17:52 ` John Fastabend 2013-12-12 19:03 ` Florian Fainelli 0 siblings, 1 reply; 15+ messages in thread From: John Fastabend @ 2013-12-12 17:52 UTC (permalink / raw) To: raspl; +Cc: David Miller, stephen, bhutchings, blaschka, netdev, linux-s390 On 12/12/2013 9:28 AM, David Miller wrote: > From: Stefan Raspl <raspl@linux.vnet.ibm.com> > Date: Thu, 12 Dec 2013 14:57:18 +0100 > >> I agree, netlink would certainly be nice. But ethtool is the >> de-facto standard for now, and there doesn't seem to be a >> netlink-based alternative in sight - or is there? >> Offering a netlink based tool seems to be a different discussion, >> and if that takes shape, existing ethtool functionality (including >> this series) can and should be migrated. > > I completely disagree, ethtool is not de-facto for anything in > particular. It's suitable for some things, not suitable for > others. > > And just because it's used for other aspects of a device's > configuration doesn't mean that netlink isn't appropriate for other > apsects. Just to elaborate... Any application using lldp information will want to get events when TLVs change. Maybe you can contrive ethtool to do this but its going to be ugly. Netlink can support multicast events and applications can register for them. Also netlink's TLV format matches nicely with LLDPs TLV format. If you push an ethtool interface now we get stuck with an interface that is only useful in a very narrow use case. And netlink interfaces are relatively easy to construct so lets do this right the first time. I also happen to maintain lldpad and would be happy to plug in support for this via netlink so we can push firmware and software based agent LLDP info up to libvirt/network manager whatever in a consistent way. Similarly other existing lldp agents could do the same. Thanks, .John ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 17:52 ` John Fastabend @ 2013-12-12 19:03 ` Florian Fainelli 2013-12-12 21:47 ` John Fastabend 0 siblings, 1 reply; 15+ messages in thread From: Florian Fainelli @ 2013-12-12 19:03 UTC (permalink / raw) To: John Fastabend Cc: raspl, David Miller, Stephen Hemminger, Ben Hutchings, blaschka, netdev, linux-s390 2013/12/12 John Fastabend <john.r.fastabend@intel.com>: > On 12/12/2013 9:28 AM, David Miller wrote: >> >> From: Stefan Raspl <raspl@linux.vnet.ibm.com> >> Date: Thu, 12 Dec 2013 14:57:18 +0100 >> >>> I agree, netlink would certainly be nice. But ethtool is the >>> de-facto standard for now, and there doesn't seem to be a >>> netlink-based alternative in sight - or is there? >>> Offering a netlink based tool seems to be a different discussion, >>> and if that takes shape, existing ethtool functionality (including >>> this series) can and should be migrated. >> >> >> I completely disagree, ethtool is not de-facto for anything in >> particular. It's suitable for some things, not suitable for >> others. >> >> And just because it's used for other aspects of a device's >> configuration doesn't mean that netlink isn't appropriate for other >> apsects. > > > Just to elaborate... > > Any application using lldp information will want to get events when > TLVs change. Maybe you can contrive ethtool to do this but its going to > be ugly. Netlink can support multicast events and applications can > register for them. Also netlink's TLV format matches nicely with LLDPs > TLV format. ethtool and netlink usually intersect for a few bits of information such as link status for instance. It is useful to have this information twice, with ethtool as a debugging aid, and via netlink to take appropriate actions. Maybe we just need to be clear on what needs to be present in ethtool only (configuration, static information) and see on a case-by-case what needs to be present in both ethtool and netlink? > > If you push an ethtool interface now we get stuck with an interface > that is only useful in a very narrow use case. And netlink interfaces > are relatively easy to construct so lets do this right the first time. > I also happen to maintain lldpad and would be happy to plug in support > for this via netlink so we can push firmware and software based agent > LLDP info up to libvirt/network manager whatever in a consistent way. > Similarly other existing lldp agents could do the same. > > Thanks, > .John -- Florian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 19:03 ` Florian Fainelli @ 2013-12-12 21:47 ` John Fastabend 2013-12-12 22:00 ` Ben Hutchings 2013-12-16 15:32 ` Stefan Raspl 0 siblings, 2 replies; 15+ messages in thread From: John Fastabend @ 2013-12-12 21:47 UTC (permalink / raw) To: Florian Fainelli Cc: raspl, David Miller, Stephen Hemminger, Ben Hutchings, blaschka, netdev, linux-s390 [...] >> Just to elaborate... >> >> Any application using lldp information will want to get events when >> TLVs change. Maybe you can contrive ethtool to do this but its going to >> be ugly. Netlink can support multicast events and applications can >> register for them. Also netlink's TLV format matches nicely with LLDPs >> TLV format. > > ethtool and netlink usually intersect for a few bits of information > such as link status for instance. It is useful to have this > information twice, with ethtool as a debugging aid, and via netlink to > take appropriate actions. > > Maybe we just need to be clear on what needs to be present in ethtool > only (configuration, static information) and see on a case-by-case > what needs to be present in both ethtool and netlink? > OK if there is an enable/disable bit in ethtool that might make some sense. Or an error flag that is helpful to have. In this case we are dealing with peer attributes which are dynamic and in my opinion should go into netlink and duplicating them in ethtool although possible doesn't seem very useful to me. .John ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 21:47 ` John Fastabend @ 2013-12-12 22:00 ` Ben Hutchings 2013-12-16 15:32 ` Stefan Raspl 1 sibling, 0 replies; 15+ messages in thread From: Ben Hutchings @ 2013-12-12 22:00 UTC (permalink / raw) To: John Fastabend Cc: Florian Fainelli, raspl, David Miller, Stephen Hemminger, blaschka, netdev, linux-s390 On Thu, 2013-12-12 at 13:47 -0800, John Fastabend wrote: > [...] > > >> Just to elaborate... > >> > >> Any application using lldp information will want to get events when > >> TLVs change. Maybe you can contrive ethtool to do this but its going to > >> be ugly. Netlink can support multicast events and applications can > >> register for them. Also netlink's TLV format matches nicely with LLDPs > >> TLV format. > > > > ethtool and netlink usually intersect for a few bits of information > > such as link status for instance. It is useful to have this > > information twice, with ethtool as a debugging aid, and via netlink to > > take appropriate actions. > > > > Maybe we just need to be clear on what needs to be present in ethtool > > only (configuration, static information) and see on a case-by-case > > what needs to be present in both ethtool and netlink? > > > > OK if there is an enable/disable bit in ethtool that might make some > sense. Or an error flag that is helpful to have. > > In this case we are dealing with peer attributes which are dynamic and > in my opinion should go into netlink and duplicating them in ethtool > although possible doesn't seem very useful to me. I agree with this - rtnetlink seems to make more sense. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes @ 2013-12-12 22:00 ` Ben Hutchings 0 siblings, 0 replies; 15+ messages in thread From: Ben Hutchings @ 2013-12-12 22:00 UTC (permalink / raw) To: John Fastabend Cc: Florian Fainelli, raspl, David Miller, Stephen Hemminger, blaschka, netdev, linux-s390 On Thu, 2013-12-12 at 13:47 -0800, John Fastabend wrote: > [...] > > >> Just to elaborate... > >> > >> Any application using lldp information will want to get events when > >> TLVs change. Maybe you can contrive ethtool to do this but its going to > >> be ugly. Netlink can support multicast events and applications can > >> register for them. Also netlink's TLV format matches nicely with LLDPs > >> TLV format. > > > > ethtool and netlink usually intersect for a few bits of information > > such as link status for instance. It is useful to have this > > information twice, with ethtool as a debugging aid, and via netlink to > > take appropriate actions. > > > > Maybe we just need to be clear on what needs to be present in ethtool > > only (configuration, static information) and see on a case-by-case > > what needs to be present in both ethtool and netlink? > > > > OK if there is an enable/disable bit in ethtool that might make some > sense. Or an error flag that is helpful to have. > > In this case we are dealing with peer attributes which are dynamic and > in my opinion should go into netlink and duplicating them in ethtool > although possible doesn't seem very useful to me. I agree with this - rtnetlink seems to make more sense. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-12 21:47 ` John Fastabend 2013-12-12 22:00 ` Ben Hutchings @ 2013-12-16 15:32 ` Stefan Raspl 2014-01-07 14:28 ` Stefan Raspl 1 sibling, 1 reply; 15+ messages in thread From: Stefan Raspl @ 2013-12-16 15:32 UTC (permalink / raw) To: John Fastabend, Florian Fainelli Cc: David Miller, Stephen Hemminger, Ben Hutchings, blaschka, netdev, linux-s390 Am 12.12.2013 22:47, schrieb John Fastabend: > [...] > >>> Just to elaborate... >>> >>> Any application using lldp information will want to get events when >>> TLVs change. Maybe you can contrive ethtool to do this but its >>> going to >>> be ugly. Netlink can support multicast events and applications can >>> register for them. Also netlink's TLV format matches nicely with >>> LLDPs >>> TLV format. >> >> ethtool and netlink usually intersect for a few bits of information >> such as link status for instance. It is useful to have this >> information twice, with ethtool as a debugging aid, and via >> netlink to >> take appropriate actions. >> >> Maybe we just need to be clear on what needs to be present in ethtool >> only (configuration, static information) and see on a case-by-case >> what needs to be present in both ethtool and netlink? >> > > OK if there is an enable/disable bit in ethtool that might make some > sense. Or an error flag that is helpful to have. > > In this case we are dealing with peer attributes which are dynamic and > in my opinion should go into netlink and duplicating them in ethtool > although possible doesn't seem very useful to me. I think what most folks in the discussion so far assume is that we have a full LLDP implementation with respective hooks and events. This is not true for our device: We can only poll it for the adjacent link port's state, and that's it. I.e. we don't receive any events for changes on the port's state. lldpad seems to handle the entire LLDP layer in software. And I'm not sure if our device integrates with that so well, as it handles LLDP on its own. We'd have to disable pretty much all functionality that lldpad offers, and limit support to display of a few parameters which we would have to poll on demand. Likewise with netlink: If one of the arguments for netlink is that it supports notifications, then we can't take any advantage of that either, for the reasons stated above. By its nature, what we can offer and support with our device is simple debugging functionality, displaying the current state of the switch port at a given moment - just like ethtool will display the current port speed and media type. Hence the original idea to add respective functionality to ethtool in a generic manner, so others with similar constraints could use it as well. If ethtool is not acceptable, and if lldpad and netlink are no good fits either, would respective sysfs attributes for our device type work? Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes 2013-12-16 15:32 ` Stefan Raspl @ 2014-01-07 14:28 ` Stefan Raspl 0 siblings, 0 replies; 15+ messages in thread From: Stefan Raspl @ 2014-01-07 14:28 UTC (permalink / raw) To: Stephen Hemminger, David Miller Cc: John Fastabend, Florian Fainelli, David Miller, Ben Hutchings, blaschka, netdev, linux-s390 On 12/16/2013 04:32 PM, Stefan Raspl wrote: > Am 12.12.2013 22:47, schrieb John Fastabend: >> [...] >> >>>> Just to elaborate... >>>> >>>> Any application using lldp information will want to get events when >>>> TLVs change. Maybe you can contrive ethtool to do this but its >>>> going to >>>> be ugly. Netlink can support multicast events and applications can >>>> register for them. Also netlink's TLV format matches nicely with >>>> LLDPs >>>> TLV format. >>> >>> ethtool and netlink usually intersect for a few bits of information >>> such as link status for instance. It is useful to have this >>> information twice, with ethtool as a debugging aid, and via >>> netlink to >>> take appropriate actions. >>> >>> Maybe we just need to be clear on what needs to be present in ethtool >>> only (configuration, static information) and see on a case-by-case >>> what needs to be present in both ethtool and netlink? >>> >> >> OK if there is an enable/disable bit in ethtool that might make some >> sense. Or an error flag that is helpful to have. >> >> In this case we are dealing with peer attributes which are dynamic and >> in my opinion should go into netlink and duplicating them in ethtool >> although possible doesn't seem very useful to me. > > I think what most folks in the discussion so far assume is that we > have a full LLDP implementation with respective hooks and events. > This is not true for our device: We can only poll it for the > adjacent link port's state, and that's it. I.e. we don't receive any > events for changes on the port's state. > lldpad seems to handle the entire LLDP layer in software. And I'm > not sure if our device integrates with that so well, as it handles > LLDP on its own. We'd have to disable pretty much all functionality > that lldpad offers, and limit support to display of a few parameters > which we would have to poll on demand. > Likewise with netlink: If one of the arguments for netlink is that > it supports notifications, then we can't take any advantage of that > either, for the reasons stated above. > By its nature, what we can offer and support with our device is > simple debugging functionality, displaying the current state of the > switch port at a given moment - just like ethtool will display the > current port speed and media type. Hence the original idea to add > respective functionality to ethtool in a generic manner, so others > with similar constraints could use it as well. > If ethtool is not acceptable, and if lldpad and netlink are no good > fits either, would respective sysfs attributes for our device type work? Since I didn't receive any further feedback, please let me summarize: * As elaborated in my most recent reply (cited above), our device does not provide for any notifications regarding LLDP-related events - we can only query the current state. Hence we could not take advantage of a netlink interface. Plus even if we still went with netlink, we'd have to introduce yet another userspace tool just for querying the current state. * For the same reason, lldpad would be hard to integrate with, since all we can do is to query a limited amount of information, where lldpad seems to be targeted at devices that can provide events. * Integration with ethtool was our attempt at providing a common interface for our and other devices with similar characteristics regarding LLDP, since ethtool is semantically a good fit. However, it was indicated that this is not desirable. * It seems that the only choice left is to implement sysfs attributes to query LLDP-related attributes. Any other device with similar characteristics would probably need to re-do the same functionality independently. Is this really what we want to do? Please let me know what you think. Regards, Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Display adjacent switch port's attributes @ 2014-01-07 14:28 ` Stefan Raspl 0 siblings, 0 replies; 15+ messages in thread From: Stefan Raspl @ 2014-01-07 14:28 UTC (permalink / raw) To: Stephen Hemminger Cc: John Fastabend, Florian Fainelli, David Miller, Ben Hutchings, blaschka, netdev, linux-s390 On 12/16/2013 04:32 PM, Stefan Raspl wrote: > Am 12.12.2013 22:47, schrieb John Fastabend: >> [...] >> >>>> Just to elaborate... >>>> >>>> Any application using lldp information will want to get events when >>>> TLVs change. Maybe you can contrive ethtool to do this but its >>>> going to >>>> be ugly. Netlink can support multicast events and applications can >>>> register for them. Also netlink's TLV format matches nicely with >>>> LLDPs >>>> TLV format. >>> >>> ethtool and netlink usually intersect for a few bits of information >>> such as link status for instance. It is useful to have this >>> information twice, with ethtool as a debugging aid, and via >>> netlink to >>> take appropriate actions. >>> >>> Maybe we just need to be clear on what needs to be present in ethtool >>> only (configuration, static information) and see on a case-by-case >>> what needs to be present in both ethtool and netlink? >>> >> >> OK if there is an enable/disable bit in ethtool that might make some >> sense. Or an error flag that is helpful to have. >> >> In this case we are dealing with peer attributes which are dynamic and >> in my opinion should go into netlink and duplicating them in ethtool >> although possible doesn't seem very useful to me. > > I think what most folks in the discussion so far assume is that we > have a full LLDP implementation with respective hooks and events. > This is not true for our device: We can only poll it for the > adjacent link port's state, and that's it. I.e. we don't receive any > events for changes on the port's state. > lldpad seems to handle the entire LLDP layer in software. And I'm > not sure if our device integrates with that so well, as it handles > LLDP on its own. We'd have to disable pretty much all functionality > that lldpad offers, and limit support to display of a few parameters > which we would have to poll on demand. > Likewise with netlink: If one of the arguments for netlink is that > it supports notifications, then we can't take any advantage of that > either, for the reasons stated above. > By its nature, what we can offer and support with our device is > simple debugging functionality, displaying the current state of the > switch port at a given moment - just like ethtool will display the > current port speed and media type. Hence the original idea to add > respective functionality to ethtool in a generic manner, so others > with similar constraints could use it as well. > If ethtool is not acceptable, and if lldpad and netlink are no good > fits either, would respective sysfs attributes for our device type work? Since I didn't receive any further feedback, please let me summarize: * As elaborated in my most recent reply (cited above), our device does not provide for any notifications regarding LLDP-related events - we can only query the current state. Hence we could not take advantage of a netlink interface. Plus even if we still went with netlink, we'd have to introduce yet another userspace tool just for querying the current state. * For the same reason, lldpad would be hard to integrate with, since all we can do is to query a limited amount of information, where lldpad seems to be targeted at devices that can provide events. * Integration with ethtool was our attempt at providing a common interface for our and other devices with similar characteristics regarding LLDP, since ethtool is semantically a good fit. However, it was indicated that this is not desirable. * It seems that the only choice left is to implement sysfs attributes to query LLDP-related attributes. Any other device with similar characteristics would probably need to re-do the same functionality independently. Is this really what we want to do? Please let me know what you think. Regards, Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-01-07 14:28 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-12-11 13:28 [PATCH 0/2] Display adjacent switch port's attributes Stefan Raspl 2013-12-11 13:28 ` [PATCH 1/2] ethtool: Add callback to indicate adjacent switch port attributes Stefan Raspl 2013-12-11 13:29 ` [PATCH 2/2] qeth: Display adjacent switch port attributes in ethtool Stefan Raspl 2013-12-11 20:13 ` [PATCH 0/2] Display adjacent switch port's attributes Stephen Hemminger 2013-12-12 10:06 ` Nicolas Dichtel 2013-12-12 13:57 ` Stefan Raspl 2013-12-12 17:28 ` David Miller 2013-12-12 17:52 ` John Fastabend 2013-12-12 19:03 ` Florian Fainelli 2013-12-12 21:47 ` John Fastabend 2013-12-12 22:00 ` Ben Hutchings 2013-12-12 22:00 ` Ben Hutchings 2013-12-16 15:32 ` Stefan Raspl 2014-01-07 14:28 ` Stefan Raspl 2014-01-07 14:28 ` Stefan Raspl
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.