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
