netdev - Re: [PATCH v2 net-next 3/7] octeontx2-vf: Virtual function driver support

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: <CA+sq2CeP3rfhBmxcs9Z6n7wVBmqP6upb8XFZF7nZ3R=QUtTF_g@mail.gmail.com>
Date:   Sat, 14 Mar 2020 21:10:28 +0530
From:   Sunil Kovvuri <sunil.kovvuri@...il.com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Tomasz Duszynski <tduszynski@...vell.com>,
        Subbaraya Sundeep <sbhatta@...vell.com>,
        Geetha sowjanya <gakula@...vell.com>,
        Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [PATCH v2 net-next 3/7] octeontx2-vf: Virtual function driver support

On Fri, Mar 13, 2020 at 11:41 PM Leon Romanovsky <leon@...nel.org> wrote:
 > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> > new file mode 100644
> > index 0000000..cf366dc
> > --- /dev/null
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> > @@ -0,0 +1,659 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Marvell OcteonTx2 RVU Virtual Function ethernet driver
> > + *
> > + * Copyright (C) 2020 Marvell International Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
>
> Please don't add license text, the SPDX line is enough.
>

Can you please point me to where this is written.
It would be great if these are made rules and written somewhere so
that everyone can go through and follow.
I see that there are so many patches being submitted with copyright text.
So this is very confusing.

> > +
> > +static int otx2vf_process_mbox_msg_up(struct otx2_nic *vf,
> > +                                   struct mbox_msghdr *req)
> > +{
> > +     /* Check if valid, if not reply with a invalid msg */
> > +     if (req->sig != OTX2_MBOX_REQ_SIG) {
> > +             otx2_reply_invalid_msg(&vf->mbox.mbox_up, 0, 0, req->id);
> > +             return -ENODEV;
> > +     }
> > +
> > +     switch (req->id) {
> > +#define M(_name, _id, _fn_name, _req_type, _rsp_type)                        \
> > +     case _id: {                                                     \
> > +             struct _rsp_type *rsp;                                  \
> > +             int err;                                                \
> > +                                                                     \
> > +             rsp = (struct _rsp_type *)otx2_mbox_alloc_msg(          \
> > +                     &vf->mbox.mbox_up, 0,                           \
> > +                     sizeof(struct _rsp_type));                      \
> > +             if (!rsp)                                               \
> > +                     return -ENOMEM;                                 \
> > +                                                                     \
> > +             rsp->hdr.id = _id;                                      \
> > +             rsp->hdr.sig = OTX2_MBOX_RSP_SIG;                       \
> > +             rsp->hdr.pcifunc = 0;                                   \
> > +             rsp->hdr.rc = 0;                                        \
> > +                                                                     \
> > +             err = otx2_mbox_up_handler_ ## _fn_name(                \
> > +                     vf, (struct _req_type *)req, rsp);              \
> > +             return err;                                             \
> > +     }
> > +MBOX_UP_CGX_MESSAGES
> > +#undef M
>
> "return ..." inside macro which is called by another macro is highly
> discouraged by the Linux kernel coding style.
>

There are many mailbox messages to handle and adding each one of them
to switch case would be a
lot of duplicate code. Hence we choose to with these macros.

Thanks,
Sunil.