netdev - Re: [PATCH 10/14] SIWv2: Transmit path: siw_qp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OF564FAAA3.FC8A4AB9-ONC12578B5.003C1E38-C12578B5.0044F0C1@ch.ibm.com>
Date:	Mon, 20 Jun 2011 14:33:00 +0200
From:	Bernard Metzler <BMT@...ich.ibm.com>
To:	Bart Van Assche <bvanassche@....org>
Cc:	bart.vanassche@...il.com, linux-rdma@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH 10/14] SIWv2: Transmit path: siw_qp_tx.c



bart.vanassche@...il.com wrote on 06/18/2011 07:47:29 PM:

> On Thu, Jun 16, 2011 at 2:42 PM, Bernard Metzler <bmt@...ich.ibm.com>
wrote:
> > ---
> >  drivers/infiniband/hw/siw/siw_qp_tx.c | 1332
> +++++++++++++++++++++++++++++++++
> >  1 files changed, 1332 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/infiniband/hw/siw/siw_qp_tx.c
> >
> > diff --git a/drivers/infiniband/hw/siw/siw_qp_tx.c
> b/drivers/infiniband/hw/siw/siw_qp_tx.c
> [ ... ]
> > +/*
> > + * Write out iov referencing hdr, data and trailer of current FPDU.
> > + * Update transmit state dependent on write return status
> > + */
> > +static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
> > +{
> > +       struct siw_wqe          *wqe = c_tx->wqe;
> > +       struct siw_sge          *sge = &wqe->wr.sgl.sge[c_tx->sge_idx],
> > +                               *first_sge = sge;
> > +       struct siw_mr           *mr = NULL;
> > +       struct ib_umem_chunk    *chunk = c_tx->umem_chunk;
> > +
> > +       struct kvec             iov[MAX_ARRAY];
> > +       struct page             *page_array[MAX_ARRAY];
> > +       struct msghdr           msg = {.msg_flags = MSG_DONTWAIT};
> > +
> > +       int                     seg = 0, do_crc = c_tx->do_crc,
> is_kva = 0, rv;
> > +       unsigned int            data_len = c_tx->bytes_unsent,
> > +                               hdr_len = 0,
> > +                               trl_len = 0,
> > +                               sge_off = c_tx->sge_off,
> > +                               sge_idx = c_tx->sge_idx,
> > +                               pg_idx = c_tx->pg_idx;
>
> Have you run the siw source code through sparse ? Sparse reports the
> following for the above:
>
yes, I did.

> drivers/infiniband/hw/siw/siw_qp_tx.c:694:1: warning: the frame size
> of 1120 bytes is larger than 1024 bytes
>
> I assume that you know that one should be careful with stack
> allocations in the kernel ?
>
yes. I will have to make the MAX_ARRAY parameter smaller. It
defines the maximum amount of elements in a vector to be pushed to
tcp_sendmsg() in one call.
In an older version of the code we pushed iwarp header, data pages
and iwarp trailer in several consecutive calls. While the code looked
cleaner, it induced a measureable performance penalty. A smaller
MAX_ARRAY will also restrict the maximum allowed scatter-gather-elements
in a work request to a smaller safe value (currently 10 SGEs),
since the code intentionally tries to pack as many SGEs
into one iwarp frame as possible.
Alternatively, the vector could be part of the queue pair
structure, but that would significantly increase per connection
memory footprint, which we wanted to avoid.

> > +               if (tx_type == RDMAP_RDMA_READ_REQ) {
>
> This is what the compiler says about the above statement:
>
> drivers/infiniband/hw/siw/siw_qp_tx.c:1145:15: warning: comparison
> between enum siw_wr_opcode and enum rdma_opcode
>
> So how can that statement be correct ?
>
it will not - must be changed to 'SIW_WR_RDMA_READ_REQ'...many thanks!
I obviously used sparse in a wrong way:
building within kernel tree with 'make C=1 CF="-Wsparse-all' does not give
me the indicated warning (neither does setting CF="-Wenum-mismatch").
Can you please advice? Thank you and sorry for the newbie question.

Bernard.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html