Re: [RFC PATCH net-next v4 03/11] net/smc: implement ID-related operations of loopback-ism

[Wen Gu]

On 2024/3/21 16:12, Jan Karcher wrote:
> On 17/03/2024 11:05, Wen Gu wrote:
> > This implements GID and CHID related operations of loopback-ism device.
> > loopback-ism acts as an ISMv2. It's GID is generated randomly by UUIDv4
> > algorithm and CHID is reserved 0xFFFF.
>
> Hi Wen Gu,
>
> Please add that loopback-ism uses an extended GID instead of a GID here aswell.

OK, will add that. Thanks!
> > Signed-off-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx>
> > ---
> >   net/smc/smc_loopback.c | 62 ++++++++++++++++++++++++++++++++++++++----
> >   net/smc/smc_loopback.h |  3 ++
> >   2 files changed, 60 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/smc/smc_loopback.c b/net/smc/smc_loopback.c
> > index e9170d86e58f..7656a2474500 100644
> > --- a/net/smc/smc_loopback.c
> > +++ b/net/smc/smc_loopback.c
> > @@ -19,11 +19,62 @@
> >   #include "smc_loopback.h"
> >   #if IS_ENABLED(CONFIG_SMC_LO)
> > +#define SMC_LO_V2_CAPABLE    0x1 /* loopback-ism acts as ISMv2 */
> > +
> >   static const char smc_lo_dev_name[] = "loopback-ism";
> >   static struct smc_lo_dev *lo_dev;
> > +static void smc_lo_generate_id(struct smc_lo_dev *ldev)
> > +{
> > +    struct smcd_gid *lgid = &ldev->local_gid;
> > +    uuid_t uuid;
> > +
> > +    uuid_gen(&uuid);
> > +    memcpy(&lgid->gid, &uuid, sizeof(lgid->gid));
> > +    memcpy(&lgid->gid_ext, (u8 *)&uuid + sizeof(lgid->gid),
> > +           sizeof(lgid->gid_ext));
> > +
> > +    ldev->chid = SMC_LO_CHID;
> > +}
>
> Minor comment. The function name implies that there is one id set whle
> there are two different ones. The chid assignment can be easily looked over. Maybe changing the function name to 
> `smc_lo_generate_ids` would prevent this. What do you think?

Agree. I will change the function name to 'smc_lo_generate_ids'. Thanks!

> Thanks
> - Jan
>
> > +
> > +static int smc_lo_query_rgid(struct smcd_dev *smcd, struct smcd_gid *rgid,
> > +                 u32 vid_valid, u32 vid)
> > +{
> > +    struct smc_lo_dev *ldev = smcd->priv;
> > +
> > +    /* rgid should be the same as lgid */
> > +    if (!ldev || rgid->gid != ldev->local_gid.gid ||
> > +        rgid->gid_ext != ldev->local_gid.gid_ext)
> > +        return -ENETUNREACH;
> > +    return 0;
> > +}
> > +
> > +static int smc_lo_supports_v2(void)
> > +{
> > +    return SMC_LO_V2_CAPABLE;
> > +}
> > +
> > +static void smc_lo_get_local_gid(struct smcd_dev *smcd,
> > +                 struct smcd_gid *smcd_gid)
> > +{
> > +    struct smc_lo_dev *ldev = smcd->priv;
> > +
> > +    smcd_gid->gid = ldev->local_gid.gid;
> > +    smcd_gid->gid_ext = ldev->local_gid.gid_ext;
> > +}
> > +
> > +static u16 smc_lo_get_chid(struct smcd_dev *smcd)
> > +{
> > +    return ((struct smc_lo_dev *)smcd->priv)->chid;
> > +}
> > +
> > +static struct device *smc_lo_get_dev(struct smcd_dev *smcd)
> > +{
> > +    return &((struct smc_lo_dev *)smcd->priv)->dev;
> > +}
> > +
> >   static const struct smcd_ops lo_ops = {
> > -    .query_remote_gid    = NULL,
> > +    .query_remote_gid = smc_lo_query_rgid,
> >       .register_dmb        = NULL,
> >       .unregister_dmb        = NULL,
> >       .add_vlan_id        = NULL,
> > @@ -32,10 +83,10 @@ static const struct smcd_ops lo_ops = {
> >       .reset_vlan_required    = NULL,
> >       .signal_event        = NULL,
> >       .move_data        = NULL,
> > -    .supports_v2        = NULL,
> > -    .get_local_gid        = NULL,
> > -    .get_chid        = NULL,
> > -    .get_dev        = NULL,
> > +    .supports_v2 = smc_lo_supports_v2,
> > +    .get_local_gid = smc_lo_get_local_gid,
> > +    .get_chid = smc_lo_get_chid,
> > +    .get_dev = smc_lo_get_dev,
> >   };
> >   static struct smcd_dev *smcd_lo_alloc_dev(const struct smcd_ops *ops,
> > @@ -95,6 +146,7 @@ static void smcd_lo_unregister_dev(struct smc_lo_dev *ldev)
> >   static int smc_lo_dev_init(struct smc_lo_dev *ldev)
> >   {
> > +    smc_lo_generate_id(ldev);
> >       return smcd_lo_register_dev(ldev);
> >   }
> > diff --git a/net/smc/smc_loopback.h b/net/smc/smc_loopback.h
> > index 9dd44d4c0ca3..55b41133a97f 100644
> > --- a/net/smc/smc_loopback.h
> > +++ b/net/smc/smc_loopback.h
> > @@ -20,10 +20,13 @@
> >   #if IS_ENABLED(CONFIG_SMC_LO)
> >   #define SMC_LO_MAX_DMBS        5000
> > +#define SMC_LO_CHID        0xFFFF
> >   struct smc_lo_dev {
> >       struct smcd_dev *smcd;
> >       struct device dev;
> > +    u16 chid;
> > +    struct smcd_gid local_gid;
> >   };
> >   #endif