RE: [net-next v2 2/8] i40e: transmit, receive, and napi

-----Original Message-----
From: Stefan Assmann [mailto:sassmann@kpanic.de]
Sent: Friday, August 23, 2013 5:42 AM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Brandeburg, Jesse; netdev@vger.kernel.org;
gospo@redhat.com; Nelson, Shannon; Waskiewicz Jr, Peter P; e1000-
devel@lists.sourceforge.net
Subject: Re: [net-next v2 2/8] i40e: transmit, receive, and napi

On 23.08.2013 04:15, Jeff Kirsher wrote:

quoted

From: Jesse Brandeburg <redacted>

This patch contains the transmit, receive, and napi routines, as well
as ancillary routines.

This file is code that is (will be) shared between the VF and PF
drivers.

Just some small nitpicks.

Thanks again.

quoted

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c

b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

quoted

new file mode 100644
index 0000000..ceafef0

--- /dev/null
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

[...]

quoted

+static void i40e_receive_skb(struct i40e_ring *rx_ring,
+			     struct sk_buff *skb, u16 vlan_tag)
+{
+	struct i40e_vsi *vsi = rx_ring->vsi;
+	struct i40e_q_vector *q_vector = rx_ring->q_vector;
+	u64 flags = vsi->back->flags;
+
+	if (vlan_tag & VLAN_VID_MASK)
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);

Suggesting __constant_htons instead of htons here.

We'll follow Dave's comment on this.

[...]

quoted

+static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
+		    u32 tx_flags, __be16 protocol, u8 *hdr_len,
+		    u64 *cd_type_cmd_tso_mss, u32 *cd_tunneling)
+{

[...]

quoted

+	cd_cmd = I40E_TX_CTX_DESC_TSO;
+	cd_tso_len = skb->len - *hdr_len;
+	cd_mss = skb_shinfo(skb)->gso_size;
+	*cd_type_cmd_tso_mss |= ((u64)cd_cmd	<<

I40E_TXD_CTX_QW1_CMD_SHIFT)

quoted

+			     | ((u64)cd_tso_len
+				<< I40E_TXD_CTX_QW1_TSO_LEN_SHIFT)
+			     | ((u64)cd_mss     << I40E_TXD_CTX_QW1_MSS_SHIFT);

Should use either tab or space after cd_cmd, cd_mss but please don't mix
them.

Thanks, we'll tweak those.

  Stefan