Hi Tomasz, Any news? Sorry to speed up, we are very very late for RC1.
2016-10-10 16:08, Thomas Monjalon: > Hi, > > Now that the feature seems to meet a consensus, I've looked at it more > closely before integrating. Sorry if it appears like a late review. > > 2016-09-30 11:00, Tomasz Kulasek: > > Added API for `rte_eth_tx_prep` > > I would love to read the usability and performance considerations here. > No need for something as long as the cover letter. Just few lines > about why it is needed and why it is a good choice for performance. > > > uint16_t rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id, > > struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > > > > Added fields to the `struct rte_eth_desc_lim`: > > > > uint16_t nb_seg_max; > > /**< Max number of segments per whole packet. */ > > > > uint16_t nb_mtu_seg_max; > > /**< Max number of segments per one MTU */ > [...] > > +#else > > + > > +static inline uint16_t > > +rte_eth_tx_prep(uint8_t port_id __rte_unused, uint16_t queue_id > > __rte_unused, > > + struct rte_mbuf **tx_pkts __rte_unused, uint16_t nb_pkts) > > Doxygen is failing here. > Have you tried to move __rte_unused before the identifier? > > [...] > > +#define PKT_TX_OFFLOAD_MASK ( \ > > + PKT_TX_IP_CKSUM | \ > > + PKT_TX_L4_MASK | \ > > + PKT_TX_OUTER_IP_CKSUM | \ > > + PKT_TX_TCP_SEG | \ > > + PKT_TX_QINQ_PKT | \ > > + PKT_TX_VLAN_PKT) > > We should really stop adding some public constants without the proper > RTE prefix. > And by the way, should not we move such flags into rte_net? Do not worry with this comment. It was just a thought which could be addressed in a separate patch by someone else. > [...] > > -SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h > > rte_sctp.h rte_icmp.h rte_arp.h > > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h > > rte_sctp.h rte_icmp.h rte_arp.h rte_pkt.h > > You can use the += operator on a new line for free :) > > No more comments, the rest seems OK. Thanks