Re: [PATCH net-next] team: add ethtool get_link_ksettings

[Hangbin Liu <liuhangbin@gmail.com>]

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