On Thu, Jul 30, 2020 at 7:28 AM Jason Wang <jasow...@redhat.com> wrote: > > > On 2020/7/29 上午12:26, Mauro Matteo Cascella wrote: > > On Tue, Jul 28, 2020 at 6:06 AM Jason Wang <jasow...@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?
Sure, I'll send a new version. Thank you. > > > > >> 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? Yeah it all depends on E1000_TXD_CMD_EOP. Besides, if not set, e1000e_tx_pkt_send() would never be called. Honestly, I don't know if this is a reasonable scenario or not. > Thanks > > > > >> Thanks > >> > >> > >>> Reported-by: Ziming Zhang <ezrak...@gmail.com> > >>> Signed-off-by: Mauro Matteo Cascella <mcasc...@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. > >>> * > -- Mauro Matteo Cascella, Red Hat Product Security 6F78 E20B 5935 928C F0A8 1A9D 4E55 23B8 BB34 10B0