Re: Auto-negotiation - Out of business



On Thu, 2016-11-24 at 12:43 +0100, Francesco Giudici wrote:

On 24/11/2016 12:08, Thomas Haller wrote:


in the past (up until today) nm-connection-editor always set duplex
to
"full", although the value was not implemented by the server. Thus,
every connection created in the past will change behavior after the
update.

to fix that, keyfile must ignore the old values like

  [ethernet]
  duplex=full

and treat them as unset.

Of course, we need a new way to encode full/half in keyfile, let's
fix
keyfile reader/writer to instead use:

  [ethernet]
  duplex=f
  duplex=h




For the speed value, there is not problem, because nm-c-e would
always
set it to 0, which is anyway what we want.



For the autonegotiation value, there is a problem:

 a)  old nm-c-e would set it to TRUE, with old libnm, old NM would
not persist (good)
 b1) old nm-c-e would set it to TRUE, with new libnm, new NM would
persist as TRUE (wrong)

nm-c-e must be fixed to not touch the default value for
autonegotiation, so it would be

 b2) new nm-c-e would not set autonet, which old libnm would not
transfer via D-Bus (and new NM wuold not persist) good
 b3) new nm-c-e would not set autonet, which new libnm would not
transfer via D-Bus (and new NM wuold not persist) good

Note that b1) cannot be fixed, although it's a supported
combination.
E.g. running nm-c-e 1.4.2 with libnm 1.5.3 against server 1.5.3.
It's a bug in nm-c-e, the solution is to upgrade to a fixed
version.


Needs fix in both nm-c-e and in keyfile reader/writer.

Instead of changing the keyfile, I would just more careful in
enforcing
the autonegotiate to manual: in case of autonegotiation disabled and
just one among speed=0 and duplex=NULL, I would ignore the link
configuration and put a warning in logs. After all, putting there
both
speed and duplex is expected when one wants to set link negotiation
manually. Otherwise, the missing settings will be random.
This would also good for b1).

Hi Francesco,

sounds good.

A warning however is not helpful, because it will emit a warning for
every existing connection out there created by nm-c-e.

nm_connection_normalize() should fixup such settings. Ideally, we would
reject them, but for sake of backward compatibility, we silently fix
them.

The documentation should mention, that speed and duplex must be set
together.

nm-c-e still needs fixing to leave the autonet, speed, and duplex
settings alone -- until it actually support setting them.


Thomas

Attachment: signature.asc
Description: This is a digitally signed message part



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