[PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number

zhenwei pi posted 3 patches 1 year, 5 months ago
[PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number
Posted by zhenwei pi 1 year, 5 months ago
Use 'sizeof(union rdma_network_hdr)' instead of hard code GRH length
for GSI and UD.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 6596a85723c9..bf8f4bc8c5c8 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -351,7 +351,7 @@ static enum resp_states rxe_resp_check_length(struct rxe_qp *qp,
 
 		for (i = 0; i < qp->resp.wqe->dma.num_sge; i++)
 			recv_buffer_len += qp->resp.wqe->dma.sge[i].length;
-		if (payload + 40 > recv_buffer_len) {
+		if (payload + sizeof(union rdma_network_hdr) > recv_buffer_len) {
 			rxe_dbg_qp(qp, "The receive buffer is too small for this UD packet.\n");
 			return RESPST_ERR_LENGTH;
 		}
-- 
2.34.1
Re: [PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number
Posted by Zhu Yanjun 1 year, 5 months ago
在 2024/8/22 14:52, zhenwei pi 写道:
> Use 'sizeof(union rdma_network_hdr)' instead of hard code GRH length
> for GSI and UD.
> 
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 6596a85723c9..bf8f4bc8c5c8 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -351,7 +351,7 @@ static enum resp_states rxe_resp_check_length(struct rxe_qp *qp,
>   
>   		for (i = 0; i < qp->resp.wqe->dma.num_sge; i++)
>   			recv_buffer_len += qp->resp.wqe->dma.sge[i].length;
> -		if (payload + 40 > recv_buffer_len) {
> +		if (payload + sizeof(union rdma_network_hdr) > recv_buffer_len) {

The definition of union rdma_network_hdr is as below

  797 union rdma_network_hdr {
  798         struct ib_grh ibgrh;
  799         struct {
  800                 /* The IB spec states that if it's IPv4, the header
  801                  * is located in the last 20 bytes of the header.
  802                  */
  803                 u8              reserved[20];
  804                 struct iphdr    roce4grh;
  805         };
  806 };

The length is 40 byte.

But in this,

sizeof(struct iphdr) (20) + sizeof(struct udphdr) (8) + sizeof(struct 
rxe_bth) (12) = 40

Not sure if we should use sizeof(union rdma_network_hdr) or 
(sizeof(struct iphdr) + sizeof(struct udphdr) + sizeof(struct rxe_bth)) 
in this place.

Best Regards,
Zhu Yanjun


>   			rxe_dbg_qp(qp, "The receive buffer is too small for this UD packet.\n");
>   			return RESPST_ERR_LENGTH;
>   		}

Re: [PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number
Posted by Jason Gunthorpe 1 year, 5 months ago
On Thu, Aug 22, 2024 at 07:59:32PM +0800, Zhu Yanjun wrote:
> 在 2024/8/22 14:52, zhenwei pi 写道:
> > Use 'sizeof(union rdma_network_hdr)' instead of hard code GRH length
> > for GSI and UD.
> > 
> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > ---
> >   drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> > index 6596a85723c9..bf8f4bc8c5c8 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> > @@ -351,7 +351,7 @@ static enum resp_states rxe_resp_check_length(struct rxe_qp *qp,
> >   		for (i = 0; i < qp->resp.wqe->dma.num_sge; i++)
> >   			recv_buffer_len += qp->resp.wqe->dma.sge[i].length;
> > -		if (payload + 40 > recv_buffer_len) {
> > +		if (payload + sizeof(union rdma_network_hdr) > recv_buffer_len) {
> 
> The definition of union rdma_network_hdr is as below
> 
>  797 union rdma_network_hdr {
>  798         struct ib_grh ibgrh;
>  799         struct {
>  800                 /* The IB spec states that if it's IPv4, the header
>  801                  * is located in the last 20 bytes of the header.
>  802                  */
>  803                 u8              reserved[20];
>  804                 struct iphdr    roce4grh;
>  805         };
>  806 };
> 
> The length is 40 byte.

This looks like the right struct to me if this is talking about the
special 40 byte blob that is placed in front of UD verbs completions.

Jason