netdev - Re: [PATCH] Propagate NCSI channel carrier loss/gain events to the kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200111.153134.978765596460592103.davem@davemloft.net>
Date:   Sat, 11 Jan 2020 15:31:34 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     johnathanx.mantey@...el.com
Cc:     netdev@...r.kernel.org, sam@...dozajonas.com
Subject: Re: [PATCH] Propagate NCSI channel carrier loss/gain events to the
 kernel

From: Johnathan Mantey <johnathanx.mantey@...el.com>
Date: Fri, 10 Jan 2020 14:02:23 -0800


Please format your Subject line as:

Subject: $SUBSYSTEM_PREFIX: Summary.

Here, $SUBSYSTEM_PREFIX would be "ncsi: "

> Problem statement:
> Insertion or removal of a network cable attached to a NCSI controlled
> network channel does not notify the kernel of the loss/gain of the
> network link.
> 
> The expectation is that /sys/class/net/eth(x)/carrier will change
> state after a pull/insertion event. In addition the carrier_up_count
> and carrier_down_count files should increment.
> 
> Change statement:
> Use the NCSI Asynchronous Event Notification handler to detect a
> change in a NCSI link.
> Add code to propagate carrier on/off state to the network interface.
> The on/off state is only modified after the existing code identifies
> if the network device HAD or HAS a link state change.

Please remove this "Problem statement:" and "Change statement:", we know
what you are talking about.

> @@ -89,6 +89,12 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv
> *ndp,
>      if ((had_link == has_link) || chained)
>          return 0;
>  
> +    if (had_link) {
> +        netif_carrier_off(ndp->ndev.dev);
> +    } else {
> +        netif_carrier_on(ndp->ndev.dev);
> +    }

As per coding style, single line basic blocks should not get curly
braces around them in this situation.

Thank you.