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

Smith, Stan stan.smith at intel.com
Tue Sep 21 08:51:23 PDT 2010


Alex Naslednikov wrote:
> Please, find attached the patch against WoF trunk

Thank you!
Looks good.

stan.
>
> Index:
> B:/users/xalex/WoF-trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp
> ===================================================================
> ---
> B:/users/xalex/WoF-trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp     (revision
>       2939) +++
>       B:/users/xalex/WoF-trunk/ulp/ipoib_NDIS6_CM/kernel/ipoib_port.cpp       (working
>       copy) @@ -4154,7 +4154,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 );
>
> @@ -4216,27 +4217,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++;
>
> -----Original Message-----
> From: Smith, Stan [mailto:stan.smith at intel.com]
> Sent: Monday, September 20, 2010 7:55 PM
> To: Alex Naslednikov; ofw at lists.openfabrics.org
> Subject: RE: [ofw] [Patch] [IPoIB_NDIS6_CM] LSO bug fix
>
> 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