Re: [PATCH 1/2] hw/net/net_tx_pkt: add function to check pkt->max_raw_frags

[Jason Wang <jasowang@redhat.com>]

On 2020/7/29 上午12:26, Mauro Matteo Cascella wrote:
> On Tue, Jul 28, 2020 at 6:06 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 2020/7/28 上午1:08, Mauro Matteo Cascella wrote:
>>> This patch introduces a new function in hw/net/net_tx_pkt.{c,h} to check the
>>> current data fragment against the maximum number of data fragments.
>>
>> I wonder whether it's better to do the check in
>> net_tx_pkt_add_raw_fragment() and fail there.
> Given the assertion, I assumed the caller is responsible for the
> check, but moving the check in net_tx_pkt_add_raw_fragment() totally
> makes sense to me.


Want to send a new version for this?


>
>> Btw, I find net_tx_pkt_add_raw_fragment() does not unmap dma when
>> returning to true, is this a bug?
> Isn't it unmapped in net_tx_pkt_reset()?


Probably but see how it was used in e1000e, the net_tx_pkt_reset() is 
only called when eop is set. Is this a bug?

Thanks

>
>> Thanks
>>
>>
>>> Reported-by: Ziming Zhang <ezrakiez@gmail.com>
>>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
>>> ---
>>>    hw/net/net_tx_pkt.c | 5 +++++
>>>    hw/net/net_tx_pkt.h | 8 ++++++++
>>>    2 files changed, 13 insertions(+)
>>>
>>> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
>>> index 9560e4a49e..d035618f2c 100644
>>> --- a/hw/net/net_tx_pkt.c
>>> +++ b/hw/net/net_tx_pkt.c
>>> @@ -400,6 +400,11 @@ bool net_tx_pkt_add_raw_fragment(struct NetTxPkt *pkt, hwaddr pa,
>>>        }
>>>    }
>>>
>>> +bool net_tx_pkt_exceed_max_fragments(struct NetTxPkt *pkt)
>>> +{
>>> +    return pkt->raw_frags >= pkt->max_raw_frags;
>>> +}
>>> +
>>>    bool net_tx_pkt_has_fragments(struct NetTxPkt *pkt)
>>>    {
>>>        return pkt->raw_frags > 0;
>>> diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h
>>> index 4ec8bbe9bd..e2ee46ae03 100644
>>> --- a/hw/net/net_tx_pkt.h
>>> +++ b/hw/net/net_tx_pkt.h
>>> @@ -179,6 +179,14 @@ bool net_tx_pkt_send_loopback(struct NetTxPkt *pkt, NetClientState *nc);
>>>     */
>>>    bool net_tx_pkt_parse(struct NetTxPkt *pkt);
>>>
>>> +/**
>>> +* indicates if the current data fragment exceeds max_raw_frags
>>> +*
>>> +* @pkt:            packet
>>> +*
>>> +*/
>>> +bool net_tx_pkt_exceed_max_fragments(struct NetTxPkt *pkt);
>>> +
>>>    /**
>>>    * indicates if there are data fragments held by this packet object.
>>>    *