Re: [PATCH for-next 2/8] RDMA/erdma: Add GID table management interfaces

[Boshi Yu <boshiyu@linux.alibaba.com>]

在 2024/11/29 16:54, Zhu Yanjun wrote:
> On 28.11.24 03:35, Boshi Yu wrote:
>> On Tue, Nov 26, 2024 at 04:51:02PM +0100, Zhu Yanjun wrote:
>>> 在 2024/11/26 7:59, Boshi Yu 写道:
>>>> The erdma_add_gid() interface inserts a GID entry at the
>>>> specified index. The erdma_del_gid() interface deletes the
>>>> GID entry at the specified index. Additionally, programs
>>>> can invoke the erdma_query_port() and erdma_get_port_immutable()
>>>> interfaces to query the GID table length.
>>>>
>>>> Signed-off-by: Boshi Yu <boshiyu@linux.alibaba.com>
>>>> Reviewed-by: Cheng Xu <chengyou@linux.alibaba.com>
>>>> ---
>>>>    drivers/infiniband/hw/erdma/erdma.h       |  1 +
>>>>    drivers/infiniband/hw/erdma/erdma_hw.h    | 28 +++++++++++-
>>>>    drivers/infiniband/hw/erdma/erdma_main.c  |  3 ++
>>>>    drivers/infiniband/hw/erdma/erdma_verbs.c | 56 ++++++++++++++++++ 
>>>> +++--
>>>>    drivers/infiniband/hw/erdma/erdma_verbs.h | 12 +++++
>>>>    5 files changed, 96 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/hw/erdma/erdma.h b/drivers/ 
>>>> infiniband/hw/erdma/erdma.h
>>>> index ad4dc1a4bdc7..42dabf674f5d 100644
>>>> --- a/drivers/infiniband/hw/erdma/erdma.h
>>>> +++ b/drivers/infiniband/hw/erdma/erdma.h
>>>> @@ -148,6 +148,7 @@ struct erdma_devattr {
>>>>        u32 max_mr;
>>>>        u32 max_pd;
>>>>        u32 max_mw;
>>>> +    u32 max_gid;
>>>>        u32 local_dma_key;
>>>>    };
>>>> diff --git a/drivers/infiniband/hw/erdma/erdma_hw.h b/drivers/ 
>>>> infiniband/hw/erdma/erdma_hw.h
>>>> index 970b392d4fb4..7e03c5f97501 100644
>>>> --- a/drivers/infiniband/hw/erdma/erdma_hw.h
>>>> +++ b/drivers/infiniband/hw/erdma/erdma_hw.h
>>>> @@ -21,6 +21,9 @@
>>>>    #define ERDMA_NUM_MSIX_VEC 32U
>>>>    #define ERDMA_MSIX_VECTOR_CMDQ 0
>>>> +/* RoCEv2 related */
>>>> +#define ERDMA_ROCEV2_GID_SIZE 16
>>>> +
>>>>    /* erdma device protocol type */
>>>>    enum erdma_proto_type {
>>>>        ERDMA_PROTO_IWARP = 0,
>>>> @@ -143,7 +146,8 @@ enum CMDQ_RDMA_OPCODE {
>>>>        CMDQ_OPCODE_DESTROY_CQ = 5,
>>>>        CMDQ_OPCODE_REFLUSH = 6,
>>>>        CMDQ_OPCODE_REG_MR = 8,
>>>> -    CMDQ_OPCODE_DEREG_MR = 9
>>>> +    CMDQ_OPCODE_DEREG_MR = 9,
>>>> +    CMDQ_OPCODE_SET_GID = 14,
>>>>    };
>>>>    enum CMDQ_COMMON_OPCODE {
>>>> @@ -401,7 +405,29 @@ struct erdma_cmdq_query_stats_resp {
>>>>        u64 rx_pps_meter_drop_packets_cnt;
>>>>    };
>>>> +enum erdma_network_type {
>>>> +    ERDMA_NETWORK_TYPE_IPV4 = 0,
>>>> +    ERDMA_NETWORK_TYPE_IPV6 = 1,
>>>> +};
>>>
>>> In the file include/rdma/ib_verbs.h
>>>
>>> "
>>> ...
>>>   183 enum rdma_network_type {
>>> ...
>>>   186     RDMA_NETWORK_IPV4,
>>>   187     RDMA_NETWORK_IPV6
>>>   188 };
>>> ...
>>> "
>>> Not sure why the above RDMA_NETWORK_IPV4 and RDMA_NETWORK_IPV6 are 
>>> not used.
>>>
>>> Zhu Yanjun
>>>
>>
>> Hi, Yanjun,
>>
>> Given that the values for RDMA_NETWORK_IPV4 and RDMA_NETWORK_IPV6 are 
>> 2 and 3,
>> respectively, we would need 2 bits to store the network type if we use 
>> them
>> directly. However, since we only need to differentiate between IPv4 
>> and IPv6
>> for the RoCEv2 protocol, 1 bit is sufficient.
> 
> I can not get you. You mean, you want to use 1 bit to differentiate 
> between IPv4 and IPv6. How to implement this idea? Can you show us the 
> difference of 1 bit (enum erdma_network_type) and 2 bits (enum 
> rdma_network_type) in driver?
> 
> Thanks,
> 
> Zhu Yanjun

Hi, Yanjun,

I'm sorry for not explaining this issue clearly. The enum 
erdma_network_type is actually a convention between the erdma hardware 
and the erdma driver. We just want to use fewer bits to pass the 
information to the hardware, independent of the kernel definition.

Thanks,

Boshi Yu

>>
>> Thanks,
>> Boshi Yu
>>
>>>> +
>>>> +enum erdma_set_gid_op {
>>>> +    ERDMA_SET_GID_OP_ADD = 0,
>>>> +    ERDMA_SET_GID_OP_DEL = 1,
>>>> +};
>>>> +
>>>> +/* set gid cfg */
>>>> +#define ERDMA_CMD_SET_GID_SGID_IDX_MASK GENMASK(15, 0)
>>>> +#define ERDMA_CMD_SET_GID_NTYPE_MASK BIT(16)
>>>> +#define ERDMA_CMD_SET_GID_OP_MASK BIT(31)
>>>> +
>>>> +struct erdma_cmdq_set_gid_req {
>>>> +    u64 hdr;
>>>> +    u32 cfg;
>>>> +    u8 gid[ERDMA_ROCEV2_GID_SIZE];
>>>> +};
>>>> +
>>>>    /* cap qword 0 definition */
>>>> +#define ERDMA_CMD_DEV_CAP_MAX_GID_MASK GENMASK_ULL(51, 48)
>>>>    #define ERDMA_CMD_DEV_CAP_MAX_CQE_MASK GENMASK_ULL(47, 40)
>>>>    #define ERDMA_CMD_DEV_CAP_FLAGS_MASK GENMASK_ULL(31, 24)
>>>>    #define ERDMA_CMD_DEV_CAP_MAX_RECV_WR_MASK GENMASK_ULL(23, 16)
>>>> diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/ 
>>>> infiniband/hw/erdma/erdma_main.c
>>>> index b6706c74cd96..d72b85e8971d 100644
>>>> --- a/drivers/infiniband/hw/erdma/erdma_main.c
>>>> +++ b/drivers/infiniband/hw/erdma/erdma_main.c
>>>> @@ -404,6 +404,7 @@ static int erdma_dev_attrs_init(struct erdma_dev 
>>>> *dev)
>>>>        dev->attrs.max_mr_size = 1ULL << ERDMA_GET_CAP(MAX_MR_SIZE, 
>>>> cap0);
>>>>        dev->attrs.max_mw = 1 << ERDMA_GET_CAP(MAX_MW, cap1);
>>>>        dev->attrs.max_recv_wr = 1 << ERDMA_GET_CAP(MAX_RECV_WR, cap0);
>>>> +    dev->attrs.max_gid = 1 << ERDMA_GET_CAP(MAX_GID, cap0);
>>>>        dev->attrs.local_dma_key = ERDMA_GET_CAP(DMA_LOCAL_KEY, cap1);
>>>>        dev->attrs.cc = ERDMA_GET_CAP(DEFAULT_CC, cap1);
>>>>        dev->attrs.max_qp = ERDMA_NQP_PER_QBLOCK * 
>>>> ERDMA_GET_CAP(QBLOCK, cap1);
>>>> @@ -482,6 +483,8 @@ static void erdma_res_cb_free(struct erdma_dev 
>>>> *dev)
>>>>    static const struct ib_device_ops erdma_device_ops_rocev2 = {
>>>>        .get_link_layer = erdma_get_link_layer,
>>>> +    .add_gid = erdma_add_gid,
>>>> +    .del_gid = erdma_del_gid,
>>>>    };
>>>>    static const struct ib_device_ops erdma_device_ops_iwarp = {
>>>> diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.c b/drivers/ 
>>>> infiniband/hw/erdma/erdma_verbs.c
>>>> index 3b7e55515cfd..9944eed584ec 100644
>>>> --- a/drivers/infiniband/hw/erdma/erdma_verbs.c
>>>> +++ b/drivers/infiniband/hw/erdma/erdma_verbs.c
>>>> @@ -367,7 +367,13 @@ int erdma_query_port(struct ib_device *ibdev, 
>>>> u32 port,
>>>>        memset(attr, 0, sizeof(*attr));
>>>> -    attr->gid_tbl_len = 1;
>>>> +    if (erdma_device_iwarp(dev)) {
>>>> +        attr->gid_tbl_len = 1;
>>>> +    } else {
>>>> +        attr->gid_tbl_len = dev->attrs.max_gid;
>>>> +        attr->ip_gids = true;
>>>> +    }
>>>> +
>>>>        attr->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_DEVICE_MGMT_SUP;
>>>>        attr->max_msg_sz = -1;
>>>> @@ -399,14 +405,14 @@ int erdma_get_port_immutable(struct ib_device 
>>>> *ibdev, u32 port,
>>>>        if (erdma_device_iwarp(dev)) {
>>>>            port_immutable->core_cap_flags = RDMA_CORE_PORT_IWARP;
>>>> +        port_immutable->gid_tbl_len = 1;
>>>>        } else {
>>>>            port_immutable->core_cap_flags =
>>>>                RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
>>>>            port_immutable->max_mad_size = IB_MGMT_MAD_SIZE;
>>>> +        port_immutable->gid_tbl_len = dev->attrs.max_gid;
>>>>        }
>>>> -    port_immutable->gid_tbl_len = 1;
>>>> -
>>>>        return 0;
>>>>    }
>>>> @@ -1853,3 +1859,47 @@ enum rdma_link_layer 
>>>> erdma_get_link_layer(struct ib_device *ibdev, u32 port_num)
>>>>    {
>>>>        return IB_LINK_LAYER_ETHERNET;
>>>>    }
>>>> +
>>>> +static int erdma_set_gid(struct erdma_dev *dev, u8 op, u32 idx,
>>>> +             const union ib_gid *gid)
>>>> +{
>>>> +    struct erdma_cmdq_set_gid_req req;
>>>> +    u8 ntype;
>>>> +
>>>> +    req.cfg = FIELD_PREP(ERDMA_CMD_SET_GID_SGID_IDX_MASK, idx) |
>>>> +          FIELD_PREP(ERDMA_CMD_SET_GID_OP_MASK, op);
>>>> +
>>>> +    if (op == ERDMA_SET_GID_OP_ADD) {
>>>> +        if (ipv6_addr_v4mapped((struct in6_addr *)gid))
>>>> +            ntype = ERDMA_NETWORK_TYPE_IPV4;
>>>> +        else
>>>> +            ntype = ERDMA_NETWORK_TYPE_IPV6;
>>>> +
>>>> +        req.cfg |= FIELD_PREP(ERDMA_CMD_SET_GID_NTYPE_MASK, ntype);
>>>> +
>>>> +        memcpy(&req.gid, gid, ERDMA_ROCEV2_GID_SIZE);
>>>> +    }
>>>> +
>>>> +    erdma_cmdq_build_reqhdr(&req.hdr, CMDQ_SUBMOD_RDMA,
>>>> +                CMDQ_OPCODE_SET_GID);
>>>> +    return erdma_post_cmd_wait(&dev->cmdq, &req, sizeof(req), NULL, 
>>>> NULL);
>>>> +}
>>>> +
>>>> +int erdma_add_gid(const struct ib_gid_attr *attr, void **context)
>>>> +{
>>>> +    struct erdma_dev *dev = to_edev(attr->device);
>>>> +    int ret;
>>>> +
>>>> +    ret = erdma_check_gid_attr(attr);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    return erdma_set_gid(dev, ERDMA_SET_GID_OP_ADD, attr->index,
>>>> +                 &attr->gid);
>>>> +}
>>>> +
>>>> +int erdma_del_gid(const struct ib_gid_attr *attr, void **context)
>>>> +{
>>>> +    return erdma_set_gid(to_edev(attr->device), ERDMA_SET_GID_OP_DEL,
>>>> +                 attr->index, NULL);
>>>> +}
>>>> diff --git a/drivers/infiniband/hw/erdma/erdma_verbs.h b/drivers/ 
>>>> infiniband/hw/erdma/erdma_verbs.h
>>>> index 90e2b35a0973..23cfeaf79eaa 100644
>>>> --- a/drivers/infiniband/hw/erdma/erdma_verbs.h
>>>> +++ b/drivers/infiniband/hw/erdma/erdma_verbs.h
>>>> @@ -326,6 +326,16 @@ static inline struct erdma_cq *to_ecq(struct 
>>>> ib_cq *ibcq)
>>>>        return container_of(ibcq, struct erdma_cq, ibcq);
>>>>    }
>>>> +static inline int erdma_check_gid_attr(const struct ib_gid_attr *attr)
>>>> +{
>>>> +    u8 ntype = rdma_gid_attr_network_type(attr);
>>>> +
>>>> +    if (ntype != RDMA_NETWORK_IPV4 && ntype != RDMA_NETWORK_IPV6)
>>>> +        return -EINVAL;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>    static inline struct erdma_user_mmap_entry *
>>>>    to_emmap(struct rdma_user_mmap_entry *ibmmap)
>>>>    {
>>>> @@ -382,5 +392,7 @@ int erdma_get_hw_stats(struct ib_device *ibdev, 
>>>> struct rdma_hw_stats *stats,
>>>>                   u32 port, int index);
>>>>    enum rdma_link_layer erdma_get_link_layer(struct ib_device *ibdev,
>>>>                          u32 port_num);
>>>> +int erdma_add_gid(const struct ib_gid_attr *attr, void **context);
>>>> +int erdma_del_gid(const struct ib_gid_attr *attr, void **context);
>>>>    #endif