linux-kernel - Re: [PATCH 3/8] NTB: Fix the default port and peer numbers for legacy drivers

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: <CAPoiz9yF1c8ybJzA1ViUkvgxo2=u9ytTn6+kYuKv1HZSsUOuWw@mail.gmail.com>
Date:   Tue, 12 Jun 2018 11:59:32 -0400
From:   Jon Mason <jdmason@...zu.us>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        linux-ntb <linux-ntb@...glegroups.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Allen Hubbe <allenbh@...il.com>,
        Serge Semin <fancer.lancer@...il.com>,
        Shyam Sundar S K <Shyam-sundar.S-k@....com>,
        Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH 3/8] NTB: Fix the default port and peer numbers for legacy drivers

On Fri, Jun 8, 2018 at 8:08 PM, Logan Gunthorpe <logang@...tatee.com> wrote:
> When the commit adding ntb_default_port_number() and
> ntb_default_peer_port_number()  entered the kernel there was no
> users of it so it was impossible to tell what the API needed.
>
> When a user finally landed a year later (ntb_pingpong) there were
> more NTB topologies were created and no consideration was considered
> to how other drivers had changed.
>
> Now that there is a user it can be fixed to provide a sensible default
> for the legacy drivers that do not implement ntb_{peer_}port_number().
> Seeing ntb_pingpong doesn't check error codes returning EINVAL was also
> not sensible.
>
> Patches for ntb_pingpong and ntb_perf follow (which are broken
> otherwise) to support hardware that doesn't have port numbers. This is
> important not only to not break support with existing drivers but for
> the cross link topology which, due to its perfect symmetry, cannot
> assign unique port numbers to each side.

This is a very long way of saying "no clients are checking the error
codes, so removing them". :)
I think the history and references to follow-on patches are not
necessary in the commit message and belong more in a 0/X.

This is more of a feature than a bug fix.  Can you break this (and the
pingpong and perf changes caused by this) off into a separate series,
as I'll want to apply this to the ntb-next and not bugfixes branch?

Thanks,
Jon

> Fixes: 1e5301196a88 ("NTB: Add indexed ports NTB API")
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> ---
>  drivers/ntb/ntb.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ntb/ntb.c b/drivers/ntb/ntb.c
> index 93f24440d11d..d955a92a095a 100644
> --- a/drivers/ntb/ntb.c
> +++ b/drivers/ntb/ntb.c
> @@ -225,10 +225,8 @@ int ntb_default_port_number(struct ntb_dev *ntb)
>         case NTB_TOPO_B2B_DSD:
>                 return NTB_PORT_SEC_DSD;
>         default:
> -               break;
> +               return 0;
>         }
> -
> -       return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_port_number);
>
> @@ -251,10 +249,8 @@ int ntb_default_peer_port_number(struct ntb_dev *ntb, int pidx)
>         case NTB_TOPO_B2B_DSD:
>                 return NTB_PORT_PRI_USD;
>         default:
> -               break;
> +               return 0;
>         }
> -
> -       return -EINVAL;
>  }
>  EXPORT_SYMBOL(ntb_default_peer_port_number);
>
> @@ -326,4 +322,3 @@ static void __exit ntb_driver_exit(void)
>         bus_unregister(&ntb_bus);
>  }
>  module_exit(ntb_driver_exit);
> -
> --
> 2.11.0
>