[4.4,9/9] net/ncsi: Improve HNCDSC AEN handler

This improves AEN handler for Host Network Controller Driver Status
Change (HNCDSC):

   * The channel's lock should be hold when accessing its state.
   * Do failover when host driver isn't ready.
   * Configure channel when host driver becomes ready.

NOTE: The first one isn't applied to the code in dev-4.4.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-aen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Hi Gavin,

On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> This improves AEN handler for Host Network Controller Driver Status
> Change (HNCDSC):
>
>    * The channel's lock should be hold when accessing its state.
>    * Do failover when host driver isn't ready.
>    * Configure channel when host driver becomes ready.
>
> NOTE: The first one isn't applied to the code in dev-4.4.

Can you clarify what you mean here?

Cheers,

Joel

>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  net/ncsi/ncsi-aen.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> index 5bc0873..26ac93d 100644
> --- a/net/ncsi/ncsi-aen.c
> +++ b/net/ncsi/ncsi-aen.c
> @@ -142,8 +142,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>         ncm = &nc->nc_modes[NCSI_MODE_LINK];
>         hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
>         ncm->ncm_data[3] = ntohl(hncdsc->status);
> -       if (ndp->ndp_active_channel != nc ||
> -           ncm->ncm_data[3] & 0x1)
> +       if (ndp->ndp_active_channel != nc)
>                 return 0;
>
>         /* If this channel is the active one and the link doesn't
> @@ -151,7 +150,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>          * The logic here is exactly similar to what we do when link
>          * is down on the active channel.
>          */
> -       ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
> +       if (!(ncm->ncm_data[3] & 0x1))
> +               ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>         ncsi_suspend_dev(nd);
>
>         return 0;
> --
> 2.1.0
>

On Fri, Oct 21, 2016 at 12:09:37PM +1030, Joel Stanley wrote:
>Hi Gavin,
>
>On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> This improves AEN handler for Host Network Controller Driver Status
>> Change (HNCDSC):
>>
>>    * The channel's lock should be hold when accessing its state.
>>    * Do failover when host driver isn't ready.
>>    * Configure channel when host driver becomes ready.
>>
>> NOTE: The first one isn't applied to the code in dev-4.4.
>
>Can you clarify what you mean here?
>

Joel, dev-4.4 is using old NCSI old where we don't have a spinlock
for NCSI channel. dev-4.7 and upstream code has one to ensure the
consistent read/update on NCSI channel'state. The commit log was
picked from the patch merged to linux.net (branch: "net"), meaning
the commit log corresponds to the code changes for upstream code.

This piece of changes isn't applied to dev-4.4. So I put a note
to clarify it. I'm too lazy and it could be more meaningful actually :)

Thanks,
Gavin

>Cheers,
>
>Joel
>
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  net/ncsi/ncsi-aen.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
>> index 5bc0873..26ac93d 100644
>> --- a/net/ncsi/ncsi-aen.c
>> +++ b/net/ncsi/ncsi-aen.c
>> @@ -142,8 +142,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>>         ncm = &nc->nc_modes[NCSI_MODE_LINK];
>>         hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
>>         ncm->ncm_data[3] = ntohl(hncdsc->status);
>> -       if (ndp->ndp_active_channel != nc ||
>> -           ncm->ncm_data[3] & 0x1)
>> +       if (ndp->ndp_active_channel != nc)
>>                 return 0;
>>
>>         /* If this channel is the active one and the link doesn't
>> @@ -151,7 +150,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>>          * The logic here is exactly similar to what we do when link
>>          * is down on the active channel.
>>          */
>> -       ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>> +       if (!(ncm->ncm_data[3] & 0x1))
>> +               ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>>         ncsi_suspend_dev(nd);
>>
>>         return 0;
>> --
>> 2.1.0
>>
>

On Fri, Oct 21, 2016 at 1:11 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> On Fri, Oct 21, 2016 at 12:09:37PM +1030, Joel Stanley wrote:
>>Hi Gavin,
>>
>>On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>> This improves AEN handler for Host Network Controller Driver Status
>>> Change (HNCDSC):
>>>
>>>    * The channel's lock should be hold when accessing its state.
>>>    * Do failover when host driver isn't ready.
>>>    * Configure channel when host driver becomes ready.
>>>
>>> NOTE: The first one isn't applied to the code in dev-4.4.
>>
>>Can you clarify what you mean here?
>>
>
> Joel, dev-4.4 is using old NCSI old where we don't have a spinlock
> for NCSI channel. dev-4.7 and upstream code has one to ensure the
> consistent read/update on NCSI channel'state. The commit log was
> picked from the patch merged to linux.net (branch: "net"), meaning
> the commit log corresponds to the code changes for upstream code.
>
> This piece of changes isn't applied to dev-4.4. So I put a note
> to clarify it. I'm too lazy and it could be more meaningful actually :)

Okay, thanks for the explanation.

Cheers,

Joel