All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhu Yanjun <yanjun.zhu@linux.dev>
To: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Zhu Yanjun <mounter625@163.com>,
	Leon Romanovsky <leon@kernel.org>
Cc: Jason Gunthorpe <jgg@nvidia.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Zhu Yanjun <zyjzyj2000@gmail.com>,
	linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH][next] RDMA/rxe: Avoid -Wflex-array-member-not-at-end warnings
Date: Fri, 19 Dec 2025 23:07:21 -0800	[thread overview]
Message-ID: <ee5a3dbe-af8c-46e5-98ea-8165fbeeeccd@linux.dev> (raw)
In-Reply-To: <ad8987ae-b7fe-47af-a1d2-5055749011c0@embeddedor.com>

在 2025/12/19 1:55, Gustavo A. R. Silva 写道:
> 
> 
> On 12/19/25 15:59, Zhu Yanjun wrote:
>>
>> 在 2025/12/18 21:48, Gustavo A. R. Silva 写道:
>>>
>>>> The struct rxe_recv_wqe is as below.
>>>>
>>>> struct rxe_recv_wqe {
>>>>      __aligned_u64       wr_id;
>>>>      __u32           reserved;
>>>>      __u32           padding;
>>>>      struct rxe_dma_info dma;
>>>
>>> Expand struct rxe_dma_info here.
>>
>> Thanks. In struct rxe_dma_info, the struct is
>>
>> struct rxe_sge {
>>         __aligned_u64 addr;
>>         __u32   length;
>>         __u32   lkey;
>> };
>>
>> But in your commit, struct ib_sge is used.
>>
>> struct ib_sge {
>>      u64 addr;
>>      u32 length;
>>      u32 lkey;
>> };
>> __aligned_u64 is a 64-bit integer with a guaranteed 8-byte alignment,
>>
>> used to preserve ABI correctness across architectures and between
>>
>> userspace and kernel, while u64 has architecture-dependent alignment.
>>
>> I am not sure if we can treate "struct rxe_sge" as the same with 
>> "struct ib_sge".
> 
> Just notice that the original code is the one actually doing that.
> See my response in this same thread:
> 
> https://lore.kernel.org/linux-hardening/d3336e9d-2b84-4698-a799- 
> b49e3845f38f@embeddedor.com/
> 
> So, if that code is fine, this is fine. If the original code is wrong,
> then that code should be fixed first.

Thanks a lot. Because struct ib_sge and struct ib_sge is different,
struct ib_sge {
     u64 addr; <--- u64 has architecture-dependent alignment
     u32 length;
     u32 lkey;
};

struct rxe_sge {
        __aligned_u64 addr;   <---guaranteed 8-byte alignment,

used to preserve ABI correctness across architectures and between

userspace and kernel

        __u32   length;
        __u32   lkey;
};

and struct rxe_sge is used in rxe_mr, it is working between userspace 
and kernel, thus, I want to keep struct rxe_mr in rxe_mr.

But in other places, I want to replace struct rxe_sge with struct 
ib_sge. The commit is as below.

In short, the commit "RDMA/rxe: Avoid -Wflex-array-member-not-at-end 
warnings" && the following commit will work well. I have made tests in 
my local host. It can work well.

Please Leon and Jason comment.

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c 
b/drivers/infiniband/sw/rxe/rxe_mr.c
index b1df05238848..390ae01f549d 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -341,7 +341,7 @@ int copy_data(
         enum rxe_mr_copy_dir    dir)
  {
         int                     bytes;
-       struct rxe_sge          *sge    = &dma->sge[dma->cur_sge];
+       struct ib_sge *sge      = &dma->sge[dma->cur_sge];
         int                     offset  = dma->sge_offset;
         int                     resid   = dma->resid;
         struct rxe_mr           *mr     = NULL;
@@ -580,7 +580,7 @@ enum resp_states rxe_mr_do_atomic_write(struct 
rxe_mr *mr, u64 iova, u64 value)

  int advance_dma_data(struct rxe_dma_info *dma, unsigned int length)
  {
-       struct rxe_sge          *sge    = &dma->sge[dma->cur_sge];
+       struct ib_sge *sge      = &dma->sge[dma->cur_sge];
         int                     offset  = dma->sge_offset;
         int                     resid   = dma->resid;

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c 
b/drivers/infiniband/sw/rxe/rxe_resp.c
index 711f73e0bbb1..74f5b695da7a 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -283,7 +283,7 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp)
                 rxe_dbg_qp(qp, "invalid num_sge in SRQ entry\n");
                 return RESPST_ERR_MALFORMED_WQE;
         }
-       size = sizeof(*wqe) + wqe->dma.num_sge*sizeof(struct rxe_sge);
+       size = sizeof(*wqe) + wqe->dma.num_sge*sizeof(struct ib_sge);
         memcpy(&qp->resp.srq_wqe, wqe, size);

         qp->resp.wqe = &qp->resp.srq_wqe.wqe;
diff --git a/include/uapi/rdma/rdma_user_rxe.h 
b/include/uapi/rdma/rdma_user_rxe.h
index bb092fccb813..360839498441 100644
--- a/include/uapi/rdma/rdma_user_rxe.h
+++ b/include/uapi/rdma/rdma_user_rxe.h
@@ -154,7 +154,7 @@ struct rxe_dma_info {
         union {
                 __DECLARE_FLEX_ARRAY(__u8, inline_data);
                 __DECLARE_FLEX_ARRAY(__u8, atomic_wr);
-               __DECLARE_FLEX_ARRAY(struct rxe_sge, sge);
+               __DECLARE_FLEX_ARRAY(struct ib_sge, sge);
         };
  };


To this commit, plus the above commit, it should work well.

Yanjun.Zhu

> 
> -Gustavo
> 
>>
>>
>> Leon and Jason, please comment on it.
>>
>>
>> Yanjun.Zhu
>>
>>>
>>>> };
>>>>
>>>> But I can not find dma.sge in the above struct. Can you explain it?
>>>>
>>>> To be honest, I read your original commit for several times, but I 
>>>> can not get it.  Can you explain the MACRO TRAILING_OVERLAP? And how 
>>>> can it replace the following struct?
>>>
>>> This is clearly explained in the changelog text. I think what you're
>>> missing will be clear once you understand how nested structures
>>> work. See my comment above.
>>>
>>> -Gustavo
>>
> 


  reply	other threads:[~2025-12-20  7:07 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11  3:35 [PATCH][next] RDMA/rxe: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2025-11-11 11:56 ` Leon Romanovsky
2025-11-11 12:14   ` Gustavo A. R. Silva
2025-11-11 14:19     ` Leon Romanovsky
2025-11-11 15:37       ` Zhu Yanjun
2025-11-12  8:49       ` Gustavo A. R. Silva
2025-11-12  9:32         ` Leon Romanovsky
2025-11-12  9:50           ` Gustavo A. R. Silva
2025-11-12 12:06             ` Leon Romanovsky
2025-12-02 18:13 ` Jason Gunthorpe
2025-12-03  7:32   ` Zhu Yanjun
2025-12-04  5:08   ` Zhu Yanjun
2025-12-04 13:05     ` Jason Gunthorpe
2025-12-04 17:48       ` yanjun.zhu
2025-12-06  4:41         ` Zhu Yanjun
2025-12-15  5:00           ` Zhu Yanjun
2025-12-18 15:56             ` Leon Romanovsky
2025-12-18 19:22               ` Yanjun.Zhu
2025-12-19  2:51                 ` Gustavo A. R. Silva
2025-12-19  4:29                   ` Zhu Yanjun
2025-12-19  4:35                     ` Gustavo A. R. Silva
2025-12-19  5:27                       ` Zhu Yanjun
2025-12-19  5:48                         ` Gustavo A. R. Silva
2025-12-19  6:59                           ` Zhu Yanjun
2025-12-19  9:55                             ` Gustavo A. R. Silva
2025-12-20  7:07                               ` Zhu Yanjun [this message]
2025-12-19 14:26                   ` Jason Gunthorpe
2025-12-26  6:13                     ` Zhu Yanjun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee5a3dbe-af8c-46e5-98ea-8165fbeeeccd@linux.dev \
    --to=yanjun.zhu@linux.dev \
    --cc=gustavo@embeddedor.com \
    --cc=gustavoars@kernel.org \
    --cc=jgg@nvidia.com \
    --cc=leon@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mounter625@163.com \
    --cc=zyjzyj2000@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.