From: Larysa Zaremba <redacted>
Date: Thu, 14 Sep 2023 18:28:07 +0200

On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:

quoted

From: Larysa Zaremba <redacted>
Date: Thu, 24 Aug 2023 21:26:49 +0200

[...]

quoted

quoted

+static void
+ice_set_rx_rings_vlan_proto(struct ice_vsi *vsi, __be16 vlan_ethertype)

@vsi can be const (I hope).

I will try to make it const.

quoted

Line can be broken on arguments, not type (I hope).

This is how we break the lines everywhere in this file though :/

I know and would really like us stop at least adding new such
occurrences when not needed :s

quoted

quoted

+{
+	u16 i;
+
+	ice_for_each_alloc_rxq(vsi, i)
+		vsi->rx_rings[i]->pkt_ctx.vlan_proto = vlan_ethertype;
+}
+
 /**
  * ice_set_vlan_offload_features - set VLAN offload features for the PF VSI
  * @vsi: PF's VSI

@@ -6049,6 +6066,11 @@ ice_set_vlan_offload_features(struct ice_vsi *vsi, netdev_features_t features)
 	if (strip_err || insert_err)
 		return -EIO;
 
+	if (enable_stripping)
+		ice_set_rx_rings_vlan_proto(vsi, htons(vlan_ethertype));
+	else
+		ice_set_rx_rings_vlan_proto(vsi, 0);

Ternary?

Would look ugly in this particular case, I think, too long expressions and no 
return values.

	ice_set_rx_rings_vlan_proto(vsi, strip ? htons(vlan_ethertype) : 0);

?

[...]

quoted

quoted

-		vlan_tag = ice_get_vlan_tag_from_rx_desc(rx_desc);
+		vlan_tci = ice_get_vlan_tci(rx_desc);

Unrelated: I never was a fan of scattering rx_desc parsing across
several files, I remember I moved it to process_skb_fields() in both ice
(Hints series) and iavf (libie), maybe do that here as well? Or way too
out of context?

A little bit too unrelated to the purpose of the series, but a thing we must do 
in the future.

Sure, +

quoted

quoted

 
 		/* pad the skb if needed, to make a valid ethernet frame */
 		if (eth_skb_pad(skb))

[...]

Thanks,
Olek

Thanks,
Olek