[ofw] [Patch] [IPoIB_NDIS6_CM] LSO bug fix

Smith, Stan stan.smith at intel.com
Mon Sep 20 10:54:33 PDT 2010


Alex Naslednikov wrote:
> Hello,
> This is an old patch that was resent as is, so this is the reason you
> can't apply (that was missed as well).
> The purpose of all these patches was:
> 1. Allow the community to review them before inserting into trunk
> 2. Remove differences between Mellanox and WoF trees
>
> So, if this patch looks ok, I will apply it by myself (using winmerge)
>
> P.S. I strictly follow our agreement that all new patches will be
> tested also on WoF trunk (even if there's no differences at all).
>
> XaleX

Thank you for the clarification of your process.
I strongly support efforts to sync the MLX svn and winOFED svn trees.

When posting a patch, some discussion of testing is helpful. Which OS/arch combo was tested, how is was tested.
We have all been a bit forgetful in explaining how we test our patches.

Reviewing patches which are not the final patch seems error prone and somewhat defeats the review process?
Since you will eventually be applying the MLX tree patch to winOFED svn prior to checkin, would it not make sense to send to the list the final patches against winOFED svn?

Otherwise you are asking the community to review patches against a src tree which the community does not have access to?
How useful, error prone and difficult is a flying blind review?

Many will ignore the patch as how can they 'really' review what's changed without seeing the original source context; eventually determining time spent on the review as wasted.


Stan.


>
>
>
> -----Original Message-----
> From: Smith, Stan [mailto:stan.smith at intel.com]
> Sent: Monday, September 20, 2010 7:23 PM
> To: Alex Naslednikov; ofw at lists.openfabrics.org
> Subject: RE: [ofw] [Patch] [IPoIB_NDIS6_CM] LSO bug fix
>
> Hello,
>   If you know the MLX ipoib files and winOFED svn files are out of
> sync, why do you posted patches which will not apply?
>
> Please resend as a patch against winOFED svn tree for consideration?
>
> thank you,
>
> stan.
>
>
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex
> Naslednikov
> Sent: Sunday, September 19, 2010 8:17 AM
> To: ofw at lists.openfabrics.org
> Subject: [ofw] [Patch] [IPoIB_NDIS6_CM] LSO bug fix
>
> LSO bug fix
>
> Signed-off by: Alexander Naslednikov (xalex  at mellanox.co.il)
>
> Index:
> B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
> ===================================================================
> ---
> B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
>         (revision 5274) +++
>
>
> B:/users/xalex/MLNX_VPI_trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
> (revision 5275) @@ -3989,7 +3989,8 @@ NDIS_STATUS             status;
> uint32_t                i, j            = 1; ULONG
> DataOffset      = 0; -       uint32_t                offset
> = sizeof(eth_hdr_t); +       uint32_t                EthHeaderOffset
> = sizeof(eth_hdr_t); +       static const    EthIPoIBHeaderOffset
> = EthHeaderOffset - sizeof(ipoib_hdr_t);
>
>         PERF_DECLARE( SendCopy );
>
> @@ -4047,27 +4048,27 @@
>                         ****************************/
>
>                         p_desc->send_wr[0].local_ds[j].vaddr =
> -                               p_sgl->Elements[0].Address.QuadPart +
> lso_header_size; +
>                         p_sgl->Elements[0].Address.QuadPart +
> lso_header_size + DataOffset + EthIPoIBHeaderOffset;
> p_desc->send_wr[0].local_ds[j].length = -
>                         p_sgl->Elements[0].Length - lso_header_size;
>                         +
>                         p_sgl->Elements[0].Length - lso_header_size -
>                         DataOffset - EthIPoIBHeaderOffset;
>                 p_desc->send_wr[0].local_ds[j].lkey =
>         s_buf->p_port->ib_mgr.lkey; /* Set the number of data
> segments. */ p_desc->send_wr[0].wr.num_ds = 1; return
> NDIS_STATUS_SUCCESS; } } -       else while( offset ) +       else
>         while( EthHeaderOffset ) {
> -               if( p_sgl->Elements[i].Length <= offset )
> +               if( p_sgl->Elements[i].Length <= EthHeaderOffset )
>                 {
> -                       offset -= p_sgl->Elements[i++].Length;
> +                       EthHeaderOffset -=
>                 p_sgl->Elements[i++].Length; }
>                 else
>                 {
>                         p_desc->send_wr[0].local_ds[j].vaddr =
> -                               p_sgl->Elements[i].Address.QuadPart +
> offset + DataOffset; +
>                         p_sgl->Elements[i].Address.QuadPart +
> EthHeaderOffset + DataOffset; p_desc->send_wr[0].local_ds[j].length =
> -                               p_sgl->Elements[i].Length - offset -
>                         DataOffset; +
>                         p_sgl->Elements[i].Length - EthHeaderOffset -
>                         DataOffset;
> p_desc->send_wr[0].local_ds[j].lkey = s_buf->p_port->ib_mgr.lkey;
> i++; j++; _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw




More information about the ofw mailing list