> On May 22, 2018, at 12:43 AM, Shahaf Shuler <shah...@mellanox.com> wrote:
> 
> Hi Koh,
> 
> Please see question below
> 
> Tuesday, May 22, 2018 9:57 AM, Yongseok Koh:
>> Subject: [PATCH v2 2/2] net/mlx5: remove redundant checks
>> 
>> Signed-off-by: Yongseok Koh <ys...@mellanox.com>
>> Acked-by: Xueming Li <xuemi...@mellanox.com>
>> ---
>> 
>> v2:
>> * add missing 'Acked-by' tag.
>> 
>> drivers/net/mlx5/mlx5_rxtx.c |  6 ++----  drivers/net/mlx5/mlx5_rxtx.h | 14
>> +++++++-------
>> 2 files changed, 9 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
>> index cdd373e3e..527859461 100644
>> --- a/drivers/net/mlx5/mlx5_rxtx.c
>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
>> @@ -522,7 +522,6 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf
>> **pkts, uint16_t pkts_n)
>>              uint16_t ehdr;
>>              uint8_t cs_flags;
>>              uint8_t tso = txq->tso_en && (buf->ol_flags &
>> PKT_TX_TCP_SEG);
>> -            uint8_t is_vlan = !!(buf->ol_flags & PKT_TX_VLAN_PKT);
>>              uint32_t swp_offsets = 0;
>>              uint8_t swp_types = 0;
>>              uint16_t tso_segsz = 0;
>> @@ -566,11 +565,10 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf
>> **pkts, uint16_t pkts_n)
>>                      rte_prefetch0(
>>                          rte_pktmbuf_mtod(*(pkts + 1), volatile void *));
>>              cs_flags = txq_ol_cksum_to_cs(buf);
>> -            txq_mbuf_to_swp(txq, buf, tso, is_vlan,
>> -                            (uint8_t *)&swp_offsets, &swp_types);
>> +            txq_mbuf_to_swp(txq, buf, (uint8_t *)&swp_offsets,
>> &swp_types);
>>              raw = ((uint8_t *)(uintptr_t)wqe) + 2 *
>> MLX5_WQE_DWORD_SIZE;
>>              /* Replace the Ethernet type by the VLAN if necessary. */
>> -            if (is_vlan) {
>> +            if (buf->ol_flags & PKT_TX_VLAN_PKT) {
>>                      uint32_t vlan = rte_cpu_to_be_32(0x81000000 |
>>                                                       buf->vlan_tci);
>>                      unsigned int len = 2 * ETHER_ADDR_LEN - 2; diff --git
>> a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index
>> 1e4b2fdb9..5d9a7dffd 100644
>> --- a/drivers/net/mlx5/mlx5_rxtx.h
>> +++ b/drivers/net/mlx5/mlx5_rxtx.h
>> @@ -651,10 +651,10 @@ mlx5_tx_dbrec(struct mlx5_txq_data *txq, volatile
>> struct mlx5_wqe *wqe)
>>  */
>> static __rte_always_inline void
>> txq_mbuf_to_swp(struct mlx5_txq_data *txq, struct rte_mbuf *buf,
>> -             uint8_t tso, uint64_t vlan,
>> -             uint8_t *offsets, uint8_t *swp_types)
>> +            uint8_t *offsets, uint8_t *swp_types)
>> {
>> -    uint64_t tunnel = buf->ol_flags & PKT_TX_TUNNEL_MASK;
>> +    const uint64_t vlan = buf->ol_flags & PKT_TX_VLAN_PKT;
>> +    const uint64_t tunnel = buf->ol_flags & PKT_TX_TUNNEL_MASK;
>>      const uint64_t csum_flags = buf->ol_flags & PKT_TX_L4_MASK;
>>      const uint64_t inner_ip =
>>              buf->ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6); @@ -663,8
>> +663,8 @@ txq_mbuf_to_swp(struct mlx5_txq_data *txq, struct rte_mbuf
>> *buf,
>>      uint16_t idx;
>>      uint16_t off;
>> 
>> -    if (likely(!tunnel || !txq->swp_en ||
>> -               (tunnel != PKT_TX_TUNNEL_UDP && tunnel !=
>> PKT_TX_TUNNEL_IP)))
>> +    if (likely(!txq->swp_en || (tunnel != PKT_TX_TUNNEL_UDP &&
>> +                                tunnel != PKT_TX_TUNNEL_IP)))
>>              return;
>>      /*
>>       * The index should have:
>> @@ -684,7 +684,7 @@ txq_mbuf_to_swp(struct mlx5_txq_data *txq, struct
>> rte_mbuf *buf,
>>       * in if any of SWP offsets is set. Therefore, all of the L3 offsets
>>       * should be set regardless of HW offload.
>>       */
>> -    off = buf->outer_l2_len + (vlan ? 4 : 0);
>> +    off = buf->outer_l2_len + (vlan ? sizeof(struct vlan_hdr) : 0);
>>      offsets[1] = off >> 1; /* Outer L3 offset. */
>>      if (tunnel == PKT_TX_TUNNEL_UDP) {
>>              off += buf->outer_l3_len;
>> @@ -693,7 +693,7 @@ txq_mbuf_to_swp(struct mlx5_txq_data *txq, struct
>> rte_mbuf *buf,
>>      if (inner_ip) {
>>              off += buf->l2_len;
>>              offsets[3] = off >> 1; /* Inner L3 offset. */
>> -            if (csum_flags == PKT_TX_TCP_CKSUM || tso ||
>> +            if (csum_flags == PKT_TX_TCP_CKSUM ||
> 
> Why are you removing the tso?
> Per my understanding application can set only the PKT_TX_TCP_SEG for TSO and 
> this implies TCP checksum. 

Right. I don't know why but I wrongly thought PKT_TX_TCP_SEG enforces 
PKT_TX_TCP_CKSUM.
Have to restore it.

Thanks,
Yongseok

>>                  csum_flags == PKT_TX_UDP_CKSUM) {
>>                      off += buf->l3_len;
>>                      offsets[2] = off >> 1; /* Inner L4 offset. */
>> --
>> 2.11.0

Reply via email to