On Sat, Nov 5, 2022 6:37 PM Li, Zhijian wrote:
> On 28/10/2022 17:04, Daisuke Matsuda wrote:
> > The function check_length() is supposed to check the length of inbound
> > packets on responder, but it actually has been a stub since the driver was
> > born. Let it check the payload length and the DMA length.
> >
> > Signed-off-by: Daisuke Matsuda <matsuda-daisuke@xxxxxxxxxxx>
> > ---
> > FOR REVIEWERS
> > I referred to IB Specification Vol 1-Revision-1.5 to create this patch.
> > Please see 9.7.4.1.6 (page.330).
> >
> > drivers/infiniband/sw/rxe/rxe_resp.c | 36 ++++++++++++++++++++++------
> > 1 file changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> > index ed5a09e86417..62e3a8195072 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> > @@ -390,16 +390,38 @@ static enum resp_states check_resource(struct rxe_qp *qp,
> > static enum resp_states check_length(struct rxe_qp *qp,
> > struct rxe_pkt_info *pkt)
> > {
> > - switch (qp_type(qp)) {
> > - case IB_QPT_RC:
> > - return RESPST_CHK_RKEY;
> > + int mtu = qp->mtu;
> > + u32 payload = payload_size(pkt);
> > + u32 dmalen = reth_len(pkt);
> >
> > - case IB_QPT_UC:
> > - return RESPST_CHK_RKEY;
> > + /* RoCEv2 packets do not have LRH.
> > + * Let's skip checking it.
> > + */
> >
> > - default:
> > - return RESPST_CHK_RKEY;
> > + if ((pkt->opcode & RXE_START_MASK) &&
> > + (pkt->opcode & RXE_END_MASK)) {
> > + /* "only" packets */
> > + if (payload > mtu)
> > + return RESPST_ERR_LENGTH;
> > +
> > + } else if ((pkt->opcode & RXE_START_MASK) ||
> > + (pkt->opcode & RXE_MIDDLE_MASK)) {
> > + /* "first" or "middle" packets */
> > + if (payload != mtu)
> > + return RESPST_ERR_LENGTH;
> > +
> > + } else if (pkt->opcode & RXE_END_MASK) {
> > + /* "last" packets */
> > + if ((pkt->paylen == 0) || (pkt->paylen > mtu))
>
> As per IBA spec, i didn't see any difference about the 'packet payload
> length' from others,
>
> so May I know why here you are using 'pkt->paylen' instead of payload ?
Thank you for taking a look.
It seems this is my mistake. I forgot to replace them with 'payload'.
I'll post v2 later.
Actually 'pkt->paylen' is larger than 'payload':
pkt->paylen = (IB BTH) + (IB payload) + (ICRC)
Daisuke
>
>
> Thanks
>
> Zhijian
>
>
>
> > + return RESPST_ERR_LENGTH;
> > + }
> > +
> > + if (pkt->opcode & (RXE_WRITE_MASK | RXE_READ_MASK)) {
> > + if (dmalen > (1 << 31))
> > + return RESPST_ERR_LENGTH;
> > }
> > +
> > + return RESPST_CHK_RKEY;
> > }
> >
> > static enum resp_states check_rkey(struct rxe_qp *qp,
