* [PATCH net-next] team: add ethtool get_link_ksettings @ 2019-05-27 3:31 Hangbin Liu 2019-05-27 17:09 ` David Miller ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Hangbin Liu @ 2019-05-27 3:31 UTC (permalink / raw) To: netdev; +Cc: Jiri Pirko, davem, Hangbin Liu Like bond, add ethtool get_link_ksettings to show the total speed. Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- drivers/net/team/team.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index 2106045b3e16..5e892ee4c006 100644 --- a/drivers/net/team/team.c +++ b/drivers/net/team/team.c @@ -2058,9 +2058,34 @@ static void team_ethtool_get_drvinfo(struct net_device *dev, strlcpy(drvinfo->version, UTS_RELEASE, sizeof(drvinfo->version)); } +static int team_ethtool_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *cmd) +{ + struct team *team= netdev_priv(dev); + unsigned long speed = 0; + struct team_port *port; + + cmd->base.duplex = DUPLEX_UNKNOWN; + cmd->base.port = PORT_OTHER; + + list_for_each_entry(port, &team->port_list, list) { + if (team_port_txable(port)) { + if (port->state.speed != SPEED_UNKNOWN) + speed += port->state.speed; + if (cmd->base.duplex == DUPLEX_UNKNOWN && + port->state.duplex != DUPLEX_UNKNOWN) + cmd->base.duplex = port->state.duplex; + } + } + cmd->base.speed = speed ? : SPEED_UNKNOWN; + + return 0; +} + static const struct ethtool_ops team_ethtool_ops = { .get_drvinfo = team_ethtool_get_drvinfo, .get_link = ethtool_op_get_link, + .get_link_ksettings = team_ethtool_get_link_ksettings, }; /*********************** -- 2.19.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-27 3:31 [PATCH net-next] team: add ethtool get_link_ksettings Hangbin Liu @ 2019-05-27 17:09 ` David Miller 2019-05-28 9:08 ` Jiri Pirko ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2019-05-27 17:09 UTC (permalink / raw) To: liuhangbin; +Cc: netdev, jiri From: Hangbin Liu <liuhangbin@gmail.com> Date: Mon, 27 May 2019 11:31:10 +0800 > Like bond, add ethtool get_link_ksettings to show the total speed. > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Yeah, this mirrors what bonding does. Jiri, please review. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-27 3:31 [PATCH net-next] team: add ethtool get_link_ksettings Hangbin Liu 2019-05-27 17:09 ` David Miller @ 2019-05-28 9:08 ` Jiri Pirko 2019-05-28 10:02 ` Hangbin Liu 2019-06-14 8:32 ` Jiri Pirko 2019-06-17 1:32 ` [PATCHv2 " Hangbin Liu 3 siblings, 1 reply; 12+ messages in thread From: Jiri Pirko @ 2019-05-28 9:08 UTC (permalink / raw) To: Hangbin Liu; +Cc: netdev, davem Mon, May 27, 2019 at 05:31:10AM CEST, liuhangbin@gmail.com wrote: >Like bond, add ethtool get_link_ksettings to show the total speed. > >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> >--- > drivers/net/team/team.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > >diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c >index 2106045b3e16..5e892ee4c006 100644 >--- a/drivers/net/team/team.c >+++ b/drivers/net/team/team.c >@@ -2058,9 +2058,34 @@ static void team_ethtool_get_drvinfo(struct net_device *dev, > strlcpy(drvinfo->version, UTS_RELEASE, sizeof(drvinfo->version)); > } > >+static int team_ethtool_get_link_ksettings(struct net_device *dev, >+ struct ethtool_link_ksettings *cmd) >+{ >+ struct team *team= netdev_priv(dev); >+ unsigned long speed = 0; >+ struct team_port *port; >+ >+ cmd->base.duplex = DUPLEX_UNKNOWN; >+ cmd->base.port = PORT_OTHER; >+ >+ list_for_each_entry(port, &team->port_list, list) { >+ if (team_port_txable(port)) { >+ if (port->state.speed != SPEED_UNKNOWN) >+ speed += port->state.speed; >+ if (cmd->base.duplex == DUPLEX_UNKNOWN && >+ port->state.duplex != DUPLEX_UNKNOWN) >+ cmd->base.duplex = port->state.duplex; What is exactly the point of this patch? Why do you need such information. This is hw-related info. If you simply sum-up all txable ports, the value is always highly misleading. For example for hash-based port selection with 2 100Mbit ports, you will get 200Mbit, but it is not true. It is up to the traffic and hash function what is the actual TX speed you can get. On the RX side, this is even more misleading as the actual speed depends on the other side of the wire. >+ } >+ } >+ cmd->base.speed = speed ? : SPEED_UNKNOWN; >+ >+ return 0; >+} >+ > static const struct ethtool_ops team_ethtool_ops = { > .get_drvinfo = team_ethtool_get_drvinfo, > .get_link = ethtool_op_get_link, >+ .get_link_ksettings = team_ethtool_get_link_ksettings, > }; > > /*********************** >-- >2.19.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-28 9:08 ` Jiri Pirko @ 2019-05-28 10:02 ` Hangbin Liu 2019-05-28 11:24 ` Jiri Pirko 0 siblings, 1 reply; 12+ messages in thread From: Hangbin Liu @ 2019-05-28 10:02 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev, davem On Tue, May 28, 2019 at 11:08:23AM +0200, Jiri Pirko wrote: > >+static int team_ethtool_get_link_ksettings(struct net_device *dev, > >+ struct ethtool_link_ksettings *cmd) > >+{ > >+ struct team *team= netdev_priv(dev); > >+ unsigned long speed = 0; > >+ struct team_port *port; > >+ > >+ cmd->base.duplex = DUPLEX_UNKNOWN; > >+ cmd->base.port = PORT_OTHER; > >+ > >+ list_for_each_entry(port, &team->port_list, list) { > >+ if (team_port_txable(port)) { > >+ if (port->state.speed != SPEED_UNKNOWN) > >+ speed += port->state.speed; > >+ if (cmd->base.duplex == DUPLEX_UNKNOWN && > >+ port->state.duplex != DUPLEX_UNKNOWN) > >+ cmd->base.duplex = port->state.duplex; > > What is exactly the point of this patch? Why do you need such > information. This is hw-related info. If you simply sum-up all txable > ports, the value is always highly misleading. > > For example for hash-based port selection with 2 100Mbit ports, > you will get 200Mbit, but it is not true. It is up to the traffic and > hash function what is the actual TX speed you can get. > On the RX side, this is even more misleading as the actual speed depends > on the other side of the wire. The number is the maximum speed in theory. I added it because someone said bond interface could show total speed while team could not... The usage is customer could get team link-speed and throughput via SNMP. Thanks Hangbin > > > >+ } > >+ } > >+ cmd->base.speed = speed ? : SPEED_UNKNOWN; > >+ > >+ return 0; > >+} > >+ > > static const struct ethtool_ops team_ethtool_ops = { > > .get_drvinfo = team_ethtool_get_drvinfo, > > .get_link = ethtool_op_get_link, > >+ .get_link_ksettings = team_ethtool_get_link_ksettings, > > }; > > > > /*********************** > >-- > >2.19.2 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-28 10:02 ` Hangbin Liu @ 2019-05-28 11:24 ` Jiri Pirko 2019-06-13 6:16 ` Hangbin Liu 0 siblings, 1 reply; 12+ messages in thread From: Jiri Pirko @ 2019-05-28 11:24 UTC (permalink / raw) To: Hangbin Liu; +Cc: netdev, davem Tue, May 28, 2019 at 12:02:11PM CEST, liuhangbin@gmail.com wrote: >On Tue, May 28, 2019 at 11:08:23AM +0200, Jiri Pirko wrote: >> >+static int team_ethtool_get_link_ksettings(struct net_device *dev, >> >+ struct ethtool_link_ksettings *cmd) >> >+{ >> >+ struct team *team= netdev_priv(dev); >> >+ unsigned long speed = 0; >> >+ struct team_port *port; >> >+ >> >+ cmd->base.duplex = DUPLEX_UNKNOWN; >> >+ cmd->base.port = PORT_OTHER; >> >+ >> >+ list_for_each_entry(port, &team->port_list, list) { >> >+ if (team_port_txable(port)) { >> >+ if (port->state.speed != SPEED_UNKNOWN) >> >+ speed += port->state.speed; >> >+ if (cmd->base.duplex == DUPLEX_UNKNOWN && >> >+ port->state.duplex != DUPLEX_UNKNOWN) >> >+ cmd->base.duplex = port->state.duplex; >> >> What is exactly the point of this patch? Why do you need such >> information. This is hw-related info. If you simply sum-up all txable >> ports, the value is always highly misleading. >> >> For example for hash-based port selection with 2 100Mbit ports, >> you will get 200Mbit, but it is not true. It is up to the traffic and >> hash function what is the actual TX speed you can get. >> On the RX side, this is even more misleading as the actual speed depends >> on the other side of the wire. > >The number is the maximum speed in theory. I added it because someone "in theory" is not what this value should return in my opinion. >said bond interface could show total speed while team could not... >The usage is customer could get team link-speed and throughput via SNMP. Has no meaning though :/ > >Thanks >Hangbin >> >> >> >+ } >> >+ } >> >+ cmd->base.speed = speed ? : SPEED_UNKNOWN; >> >+ >> >+ return 0; >> >+} >> >+ >> > static const struct ethtool_ops team_ethtool_ops = { >> > .get_drvinfo = team_ethtool_get_drvinfo, >> > .get_link = ethtool_op_get_link, >> >+ .get_link_ksettings = team_ethtool_get_link_ksettings, >> > }; >> > >> > /*********************** >> >-- >> >2.19.2 >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-28 11:24 ` Jiri Pirko @ 2019-06-13 6:16 ` Hangbin Liu 2019-06-14 8:32 ` Jiri Pirko 0 siblings, 1 reply; 12+ messages in thread From: Hangbin Liu @ 2019-06-13 6:16 UTC (permalink / raw) To: Jiri Pirko; +Cc: netdev, davem On Tue, May 28, 2019 at 01:24:31PM +0200, Jiri Pirko wrote: > Tue, May 28, 2019 at 12:02:11PM CEST, liuhangbin@gmail.com wrote: > >On Tue, May 28, 2019 at 11:08:23AM +0200, Jiri Pirko wrote: > >> >+static int team_ethtool_get_link_ksettings(struct net_device *dev, > >> >+ struct ethtool_link_ksettings *cmd) > >> >+{ > >> >+ struct team *team= netdev_priv(dev); > >> >+ unsigned long speed = 0; > >> >+ struct team_port *port; > >> >+ > >> >+ cmd->base.duplex = DUPLEX_UNKNOWN; > >> >+ cmd->base.port = PORT_OTHER; > >> >+ > >> >+ list_for_each_entry(port, &team->port_list, list) { > >> >+ if (team_port_txable(port)) { > >> >+ if (port->state.speed != SPEED_UNKNOWN) > >> >+ speed += port->state.speed; > >> >+ if (cmd->base.duplex == DUPLEX_UNKNOWN && > >> >+ port->state.duplex != DUPLEX_UNKNOWN) > >> >+ cmd->base.duplex = port->state.duplex; > >> > >> What is exactly the point of this patch? Why do you need such > >> information. This is hw-related info. If you simply sum-up all txable > >> ports, the value is always highly misleading. > >> > >> For example for hash-based port selection with 2 100Mbit ports, > >> you will get 200Mbit, but it is not true. It is up to the traffic and > >> hash function what is the actual TX speed you can get. > >> On the RX side, this is even more misleading as the actual speed depends > >> on the other side of the wire. > > > >The number is the maximum speed in theory. I added it because someone Hi Jiri, Sorry for the late reply. > > "in theory" is not what this value should return in my opinion. Would you please give some hits about what "in theory" value we should return? In my understanding, it just shows the maximum in theory speed. Just like a NIC card shows the speed 1000Mb/s, but the actually max speed could be only 700-900 Mb/s for tcp/udp testing. No need to say if the other side's max speed is only 100Mb/s, we will get lower speed value. So I think with ab, rr, lb, random mode, the team speed could be the summary of total active ports' speed. The only controversial mode may be the broadcast mode as it just broadcast all the data from all ports. But it do send all the data. If we ignore the fault tolerance sutff, all the bandwidth are used. The speed shows the total number of all NICs looks also make sense. Hope I made it clear and you could got what I mean.. > > > >said bond interface could show total speed while team could not... > >The usage is customer could get team link-speed and throughput via SNMP. > > Has no meaning though :/ Anyway, the customer is looking for this feature. Shouldn't we consider about the requirement? Thanks Hangbin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-06-13 6:16 ` Hangbin Liu @ 2019-06-14 8:32 ` Jiri Pirko 2019-06-14 15:55 ` David Miller 0 siblings, 1 reply; 12+ messages in thread From: Jiri Pirko @ 2019-06-14 8:32 UTC (permalink / raw) To: Hangbin Liu; +Cc: netdev, davem Thu, Jun 13, 2019 at 08:16:49AM CEST, liuhangbin@gmail.com wrote: >On Tue, May 28, 2019 at 01:24:31PM +0200, Jiri Pirko wrote: >> Tue, May 28, 2019 at 12:02:11PM CEST, liuhangbin@gmail.com wrote: >> >On Tue, May 28, 2019 at 11:08:23AM +0200, Jiri Pirko wrote: >> >> >+static int team_ethtool_get_link_ksettings(struct net_device *dev, >> >> >+ struct ethtool_link_ksettings *cmd) >> >> >+{ >> >> >+ struct team *team= netdev_priv(dev); >> >> >+ unsigned long speed = 0; >> >> >+ struct team_port *port; >> >> >+ >> >> >+ cmd->base.duplex = DUPLEX_UNKNOWN; >> >> >+ cmd->base.port = PORT_OTHER; >> >> >+ >> >> >+ list_for_each_entry(port, &team->port_list, list) { >> >> >+ if (team_port_txable(port)) { >> >> >+ if (port->state.speed != SPEED_UNKNOWN) >> >> >+ speed += port->state.speed; >> >> >+ if (cmd->base.duplex == DUPLEX_UNKNOWN && >> >> >+ port->state.duplex != DUPLEX_UNKNOWN) >> >> >+ cmd->base.duplex = port->state.duplex; >> >> >> >> What is exactly the point of this patch? Why do you need such >> >> information. This is hw-related info. If you simply sum-up all txable >> >> ports, the value is always highly misleading. >> >> >> >> For example for hash-based port selection with 2 100Mbit ports, >> >> you will get 200Mbit, but it is not true. It is up to the traffic and >> >> hash function what is the actual TX speed you can get. >> >> On the RX side, this is even more misleading as the actual speed depends >> >> on the other side of the wire. >> > >> >The number is the maximum speed in theory. I added it because someone > >Hi Jiri, > >Sorry for the late reply. > >> >> "in theory" is not what this value should return in my opinion. > >Would you please give some hits about what "in theory" value we should return? > >In my understanding, it just shows the maximum in theory speed. Just like a >NIC card shows the speed 1000Mb/s, but the actually max speed could be only >700-900 Mb/s for tcp/udp testing. No need to say if the other side's max speed >is only 100Mb/s, we will get lower speed value. > >So I think with ab, rr, lb, random mode, the team speed could be the summary of >total active ports' speed. The only controversial mode may be the broadcast >mode as it just broadcast all the data from all ports. But it do send all the >data. If we ignore the fault tolerance sutff, all the bandwidth are used. The >speed shows the total number of all NICs looks also make sense. > >Hope I made it clear and you could got what I mean.. Yeah, I was thinking about this in the meantime and I admit this is probably the best of the wrong approaches. The only correct one would be to teach the userspace apps to understand the topology and report parameters according to the runner, utilization, division of the traffic etc. Not real. Long story short, I'm okay with your patch. Thanks! >> >> >> >said bond interface could show total speed while team could not... >> >The usage is customer could get team link-speed and throughput via SNMP. >> >> Has no meaning though :/ > >Anyway, the customer is looking for this feature. Shouldn't we >consider about the requirement? > >Thanks >Hangbin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-06-14 8:32 ` Jiri Pirko @ 2019-06-14 15:55 ` David Miller 0 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2019-06-14 15:55 UTC (permalink / raw) To: jiri; +Cc: liuhangbin, netdev From: Jiri Pirko <jiri@resnulli.us> Date: Fri, 14 Jun 2019 10:32:25 +0200 > Long story short, I'm okay with your patch. Thanks! This patch should therefore be reposted. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] team: add ethtool get_link_ksettings 2019-05-27 3:31 [PATCH net-next] team: add ethtool get_link_ksettings Hangbin Liu 2019-05-27 17:09 ` David Miller 2019-05-28 9:08 ` Jiri Pirko @ 2019-06-14 8:32 ` Jiri Pirko 2019-06-17 1:32 ` [PATCHv2 " Hangbin Liu 3 siblings, 0 replies; 12+ messages in thread From: Jiri Pirko @ 2019-06-14 8:32 UTC (permalink / raw) To: Hangbin Liu; +Cc: netdev, davem Mon, May 27, 2019 at 05:31:10AM CEST, liuhangbin@gmail.com wrote: >Like bond, add ethtool get_link_ksettings to show the total speed. > >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Acked-by: Jiri Pirko <jiri@mellanox.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 net-next] team: add ethtool get_link_ksettings 2019-05-27 3:31 [PATCH net-next] team: add ethtool get_link_ksettings Hangbin Liu ` (2 preceding siblings ...) 2019-06-14 8:32 ` Jiri Pirko @ 2019-06-17 1:32 ` Hangbin Liu 2019-06-17 9:03 ` Jiri Pirko 2019-06-17 20:23 ` David Miller 3 siblings, 2 replies; 12+ messages in thread From: Hangbin Liu @ 2019-06-17 1:32 UTC (permalink / raw) To: netdev; +Cc: Jiri Pirko, David Miller, Hangbin Liu Like bond, add ethtool get_link_ksettings to show the total speed. v2: no update, just repost. Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- drivers/net/team/team.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index b48006e7fa2f..f3422f85f604 100644 --- a/drivers/net/team/team.c +++ b/drivers/net/team/team.c @@ -2054,9 +2054,34 @@ static void team_ethtool_get_drvinfo(struct net_device *dev, strlcpy(drvinfo->version, UTS_RELEASE, sizeof(drvinfo->version)); } +static int team_ethtool_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *cmd) +{ + struct team *team= netdev_priv(dev); + unsigned long speed = 0; + struct team_port *port; + + cmd->base.duplex = DUPLEX_UNKNOWN; + cmd->base.port = PORT_OTHER; + + list_for_each_entry(port, &team->port_list, list) { + if (team_port_txable(port)) { + if (port->state.speed != SPEED_UNKNOWN) + speed += port->state.speed; + if (cmd->base.duplex == DUPLEX_UNKNOWN && + port->state.duplex != DUPLEX_UNKNOWN) + cmd->base.duplex = port->state.duplex; + } + } + cmd->base.speed = speed ? : SPEED_UNKNOWN; + + return 0; +} + static const struct ethtool_ops team_ethtool_ops = { .get_drvinfo = team_ethtool_get_drvinfo, .get_link = ethtool_op_get_link, + .get_link_ksettings = team_ethtool_get_link_ksettings, }; /*********************** -- 2.19.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv2 net-next] team: add ethtool get_link_ksettings 2019-06-17 1:32 ` [PATCHv2 " Hangbin Liu @ 2019-06-17 9:03 ` Jiri Pirko 2019-06-17 20:23 ` David Miller 1 sibling, 0 replies; 12+ messages in thread From: Jiri Pirko @ 2019-06-17 9:03 UTC (permalink / raw) To: Hangbin Liu; +Cc: netdev, David Miller Mon, Jun 17, 2019 at 03:32:55AM CEST, liuhangbin@gmail.com wrote: >Like bond, add ethtool get_link_ksettings to show the total speed. > >v2: no update, just repost. > >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Acked-by: Jiri Pirko <jiri@mellanox.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2 net-next] team: add ethtool get_link_ksettings 2019-06-17 1:32 ` [PATCHv2 " Hangbin Liu 2019-06-17 9:03 ` Jiri Pirko @ 2019-06-17 20:23 ` David Miller 1 sibling, 0 replies; 12+ messages in thread From: David Miller @ 2019-06-17 20:23 UTC (permalink / raw) To: liuhangbin; +Cc: netdev, jiri From: Hangbin Liu <liuhangbin@gmail.com> Date: Mon, 17 Jun 2019 09:32:55 +0800 > Like bond, add ethtool get_link_ksettings to show the total speed. > > v2: no update, just repost. > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Applied, thank you. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-06-17 20:23 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-27 3:31 [PATCH net-next] team: add ethtool get_link_ksettings Hangbin Liu 2019-05-27 17:09 ` David Miller 2019-05-28 9:08 ` Jiri Pirko 2019-05-28 10:02 ` Hangbin Liu 2019-05-28 11:24 ` Jiri Pirko 2019-06-13 6:16 ` Hangbin Liu 2019-06-14 8:32 ` Jiri Pirko 2019-06-14 15:55 ` David Miller 2019-06-14 8:32 ` Jiri Pirko 2019-06-17 1:32 ` [PATCHv2 " Hangbin Liu 2019-06-17 9:03 ` Jiri Pirko 2019-06-17 20:23 ` David Miller
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.