Re: [bug report] sfc/siena: Remove build references to missing functionality

[Martin Habets <habetsm.xilinx@gmail.com>]

Hi Dan,

The analysis is correct. I'll post a patch for this.

Thanks!
Martin

On Mon, May 16, 2022 at 10:13:56AM +0300, Dan Carpenter wrote:
> Hello Martin Habets,
> 
> The patch 956f2d86cb37: "sfc/siena: Remove build references to
> missing functionality" from May 9, 2022, leads to the following
> Smatch static checker warning:
> 
> 	drivers/net/ethernet/sfc/siena/tx.c:184 __efx_siena_enqueue_skb()
> 	warn: duplicate check 'segments' (previous on line 158)
> 
> drivers/net/ethernet/sfc/siena/tx.c
>     139 netdev_tx_t __efx_siena_enqueue_skb(struct efx_tx_queue *tx_queue,
>     140                                     struct sk_buff *skb)
>     141 {
>     142         unsigned int old_insert_count = tx_queue->insert_count;
>     143         bool xmit_more = netdev_xmit_more();
>     144         bool data_mapped = false;
>     145         unsigned int segments;
>     146         unsigned int skb_len;
>     147         int rc;
>     148 
>     149         skb_len = skb->len;
>     150         segments = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 0;
>     151         if (segments == 1)
>     152                 segments = 0; /* Don't use TSO for a single segment. */
>     153 
>     154         /* Handle TSO first - it's *possible* (although unlikely) that we might
>     155          * be passed a packet to segment that's smaller than the copybreak/PIO
>     156          * size limit.
>     157          */
>     158         if (segments) {
>     159                 rc = efx_siena_tx_tso_fallback(tx_queue, skb);
>     160                 tx_queue->tso_fallbacks++;
>     161                 if (rc == 0)
>     162                         return 0;
>     163                 goto err;
> 
> If segments is non-zero then we hit this goto.
> 
>     164         } else if (skb->data_len && skb_len <= EFX_TX_CB_SIZE) {
>     165                 /* Pad short packets or coalesce short fragmented packets. */
>     166                 if (efx_enqueue_skb_copy(tx_queue, skb))
>     167                         goto err;
>     168                 tx_queue->cb_packets++;
>     169                 data_mapped = true;
>     170         }
>     171 
>     172         /* Map for DMA and create descriptors if we haven't done so already. */
>     173         if (!data_mapped && (efx_siena_tx_map_data(tx_queue, skb, segments)))
>     174                 goto err;
>     175 
>     176         efx_tx_maybe_stop_queue(tx_queue);
>     177 
>     178         tx_queue->xmit_pending = true;
>     179 
>     180         /* Pass off to hardware */
>     181         if (__netdev_tx_sent_queue(tx_queue->core_txq, skb_len, xmit_more))
>     182                 efx_tx_send_pending(tx_queue->channel);
>     183 
> --> 184         if (segments) {
> 
> So this if statement can be deleted.
> 
>     185                 tx_queue->tso_bursts++;
>     186                 tx_queue->tso_packets += segments;
>     187                 tx_queue->tx_packets  += segments;
>     188         } else {
>     189                 tx_queue->tx_packets++;
>     190         }
>     191 
>     192         return NETDEV_TX_OK;
>     193 
>     194 
>     195 err:
>     196         efx_siena_enqueue_unwind(tx_queue, old_insert_count);
>     197         dev_kfree_skb_any(skb);
>     198 
>     199         /* If we're not expecting another transmit and we had something to push
>     200          * on this queue or a partner queue then we need to push here to get the
>     201          * previous packets out.
>     202          */
>     203         if (!xmit_more)
>     204                 efx_tx_send_pending(tx_queue->channel);
>     205 
>     206         return NETDEV_TX_OK;
>     207 }
> 
> regards,
> dan carpenter