Hi > -----Original Message----- > From: Bie, Tiwei > Sent: Saturday, January 6, 2018 1:57 AM > To: Wang, Xiao W <xiao.w.w...@intel.com> > Cc: dev@dpdk.org; y...@fridaylinux.org; step...@networkplumber.org > Subject: Re: [PATCH v5 3/3] net/virtio: support GUEST ANNOUNCE > > On Fri, Jan 05, 2018 at 08:46:57AM -0800, Xiao Wang wrote: > [...] > > +static int > > +make_rarp_packet(struct rte_mbuf *rarp_mbuf, const struct ether_addr > *mac) > > +{ > > + struct ether_hdr *eth_hdr; > > + struct arp_hdr *rarp; > > Please just use one space between the type and var instead of two.
Yes. > > > + > [...] > > +static void > > +virtio_notify_peers(struct rte_eth_dev *dev) > > +{ > > + struct virtio_hw *hw = dev->data->dev_private; > > + struct virtnet_rx *rxvq = dev->data->rx_queues[0]; > > + struct rte_mbuf *rarp_mbuf; > > + > > + rarp_mbuf = rte_mbuf_raw_alloc(rxvq->mpool); > > It's not necessary to use rte_mbuf_raw_alloc() here and > you forgot to initialize the allocated mbuf. I think you > can use rte_pktmbuf_alloc() directly as what I showed in > the example in my previous mail. You are right. > > > + if (rarp_mbuf == NULL) { > > + PMD_DRV_LOG(ERR, "first mbuf allocate free_bufed"); > > Typos: > first? > free_bufed? Sorry for typo. > > > + return; > > + } > [...] > > +static void > > +virtio_ack_link_announce(struct rte_eth_dev *dev) > > +{ > > + struct virtio_hw *hw = dev->data->dev_private; > > + struct virtio_pmd_ctrl ctrl; > > + int len; > > + > > + ctrl.hdr.class = VIRTIO_NET_CTRL_ANNOUNCE; > > + ctrl.hdr.cmd = VIRTIO_NET_CTRL_ANNOUNCE_ACK; > > + len = 0; > > + > > + virtio_send_command(hw->cvq, &ctrl, &len, 0); > > If the last param is 0, then the third param could be NULL, > i.e. you don't need to define `len`. > Just checked the code, when pkt_num is 0, len field won't be used. Will make it NULL in v6. > > +} > > + > [...] > > diff --git a/drivers/net/virtio/virtio_ethdev.h > b/drivers/net/virtio/virtio_ethdev.h > > index 4a2a2f0..04b6a37 100644 > > --- a/drivers/net/virtio/virtio_ethdev.h > > +++ b/drivers/net/virtio/virtio_ethdev.h > > @@ -68,6 +68,7 @@ > > 1u << VIRTIO_NET_F_HOST_TSO6 | \ > > 1u << VIRTIO_NET_F_MRG_RXBUF | \ > > 1u << VIRTIO_NET_F_MTU | \ > > + 1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE | \ > > Please use one space before '|' instead of two. Yes, will keep it aligned with the above lines. Thanks a lot, Xiao