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

Smith, Stan stan.smith at intel.com
Tue Sep 21 09:00:29 PDT 2010


Alex Naslednikov wrote:
> Thanks for the fast answer !
> May I commit it ?

Yes, please do so.

>
> -----Original Message-----
> From: Smith, Stan [mailto:stan.smith at intel.com]
> Sent: Tuesday, September 21, 2010 5:51 PM
> To: Alex Naslednikov; ofw at lists.openfabrics.org
> Subject: RE: [ofw] [Patch] [IPoIB_NDIS6_CM] LSO bug fix
>
> 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