On Thu, Feb 2, 2017 at 2:56 PM, Jakub Kicinski <kubak...@wp.pl> wrote:
> On Thu, 2 Feb 2017 11:55:29 -0500, Michael Chan wrote:
>> @@ -755,8 +757,8 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi
>> *bnapi, u16 cp_cons,
>>
>> static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>> struct bnxt_rx_ring_info *rxr, u16 cons,
>> - u16 prod, u8 *data, dma_addr_t dma_addr,
>> - unsigned int len)
>> + u16 prod, void *data, dma_addr_t dma_addr,
>> + unsigned int offset_and_len)
>> {
>> int err;
>> struct sk_buff *skb;
>> @@ -776,7 +778,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>> }
>>
>> skb_reserve(skb, BNXT_RX_OFFSET);
>> - skb_put(skb, len);
>> + skb_put(skb, offset_and_len & 0xffff);
>> return skb;
>> }
>>
>
> Sorry to be a pain but I still don't understand (a) why you make this
> change in the first patch if it's only needed from patch 5 on;
Because I'm thinking ahead. The function is now a function pointer
and I want to make the parameter change now.
> (b) why
> do you encode the two parameters in a single u32? It's the seventh
> parameter so it's going on the stack anyway, no?
Both the length and the offset come from the hardware's rx completion
record. Both are u16. The offset happens to be in the upper 16-bit
in the hardware record. So it is convenient to encode it like this
and I chose to do it like this. Of course, using a separate parameter
will also work.
