* [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when FCoE+SRIOV are enabled
@ 2015-12-07 21:43 Neerav Parikh
2015-12-08 16:58 ` Alexander Duyck
0 siblings, 1 reply; 3+ messages in thread
From: Neerav Parikh @ 2015-12-07 21:43 UTC (permalink / raw)
To: intel-wired-lan
When FCoE is enabled with SR-IOV on the X550 NIC the hardware
generates MDD events.
This patch fixes these by setting the expected values in the
Tx context descriptors for FCoE/FIP frames and adding a flush
after writing the RDLEN register.
Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 5 ++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 5f98870..78ffef4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -517,6 +517,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
u32 vlan_macip_lens;
u32 fcoe_sof_eof = 0;
u32 mss_l4len_idx;
+ u32 type_tucmd = IXGBE_ADVTXT_TUCMD_FCOE;
u8 sof, eof;
if (skb_is_gso(skb) && (skb_shinfo(skb)->gso_type != SKB_GSO_FCOE)) {
@@ -593,6 +594,8 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
skb_shinfo(skb)->gso_size);
first->bytecount += (first->gso_segs - 1) * *hdr_len;
first->tx_flags |= IXGBE_TX_FLAGS_TSO;
+ /* Hardware expects L4T to be RSV for FCoE TSO */
+ type_tucmd |= IXGBE_ADVTXD_TUCMD_L4T_RSV;
}
/* set flag indicating FCOE to ixgbe_tx_map call */
@@ -610,7 +613,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
/* write context desc */
ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof,
- IXGBE_ADVTXT_TUCMD_FCOE, mss_l4len_idx);
+ type_tucmd, mss_l4len_idx);
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ebd4522..c31ed4c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3619,6 +3619,9 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter *adapter,
IXGBE_WRITE_REG(hw, IXGBE_RDBAH(reg_idx), (rdba >> 32));
IXGBE_WRITE_REG(hw, IXGBE_RDLEN(reg_idx),
ring->count * sizeof(union ixgbe_adv_rx_desc));
+ /* Force flushing of IXGBE_RDLEN to prevent MDD */
+ IXGBE_WRITE_FLUSH(hw);
+
IXGBE_WRITE_REG(hw, IXGBE_RDH(reg_idx), 0);
IXGBE_WRITE_REG(hw, IXGBE_RDT(reg_idx), 0);
ring->tail = adapter->io_addr + IXGBE_RDT(reg_idx);
@@ -7641,6 +7644,21 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
goto out_drop;
goto xmit_fcoe;
+ } else if (protocol == htons(ETH_P_FIP)) {
+ /* A zero as offset will result in HW generating malicious event
+ * when trying to Tx a FIP frame. So, set a valid header length
+ * to prevent such an event.
+ */
+ if (!skb_network_offset(skb)) {
+ if (tx_flags & (IXGBE_TX_FLAGS_HW_VLAN |
+ IXGBE_TX_FLAGS_SW_VLAN))
+ skb_set_network_header(skb,
+ sizeof(struct ethhdr) +
+ sizeof(struct vlan_hdr));
+ else
+ skb_set_network_header(skb,
+ sizeof(struct ethhdr));
+ }
}
#endif /* IXGBE_FCOE */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 1329edd..e93c264 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -2780,6 +2780,7 @@ struct ixgbe_adv_tx_context_desc {
#define IXGBE_ADVTXD_TUCMD_L4T_UDP 0x00000000 /* L4 Packet TYPE of UDP */
#define IXGBE_ADVTXD_TUCMD_L4T_TCP 0x00000800 /* L4 Packet TYPE of TCP */
#define IXGBE_ADVTXD_TUCMD_L4T_SCTP 0x00001000 /* L4 Packet TYPE of SCTP */
+#define IXGBE_ADVTXD_TUCMD_L4T_RSV 0x00001800 /* RSV L4 Packet TYPE */
#define IXGBE_ADVTXD_TUCMD_MKRREQ 0x00002000 /*Req requires Markers and CRC*/
#define IXGBE_ADVTXD_POPTS_IPSEC 0x00000400 /* IPSec offload request */
#define IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP 0x00002000 /* IPSec Type ESP */
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when FCoE+SRIOV are enabled
2015-12-07 21:43 [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when FCoE+SRIOV are enabled Neerav Parikh
@ 2015-12-08 16:58 ` Alexander Duyck
2015-12-08 18:24 ` Parikh, Neerav
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Duyck @ 2015-12-08 16:58 UTC (permalink / raw)
To: intel-wired-lan
On 12/07/2015 01:43 PM, Neerav Parikh wrote:
> When FCoE is enabled with SR-IOV on the X550 NIC the hardware
> generates MDD events.
>
> This patch fixes these by setting the expected values in the
> Tx context descriptors for FCoE/FIP frames and adding a flush
> after writing the RDLEN register.
>
> Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 5 ++++-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 5f98870..78ffef4 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -517,6 +517,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
> u32 vlan_macip_lens;
> u32 fcoe_sof_eof = 0;
> u32 mss_l4len_idx;
> + u32 type_tucmd = IXGBE_ADVTXT_TUCMD_FCOE;
> u8 sof, eof;
>
> if (skb_is_gso(skb) && (skb_shinfo(skb)->gso_type != SKB_GSO_FCOE)) {
> @@ -593,6 +594,8 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
> skb_shinfo(skb)->gso_size);
> first->bytecount += (first->gso_segs - 1) * *hdr_len;
> first->tx_flags |= IXGBE_TX_FLAGS_TSO;
> + /* Hardware expects L4T to be RSV for FCoE TSO */
> + type_tucmd |= IXGBE_ADVTXD_TUCMD_L4T_RSV;
> }
>
> /* set flag indicating FCOE to ixgbe_tx_map call */
> @@ -610,7 +613,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
>
> /* write context desc */
> ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof,
> - IXGBE_ADVTXT_TUCMD_FCOE, mss_l4len_idx);
> + type_tucmd, mss_l4len_idx);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ebd4522..c31ed4c 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -3619,6 +3619,9 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter *adapter,
> IXGBE_WRITE_REG(hw, IXGBE_RDBAH(reg_idx), (rdba >> 32));
> IXGBE_WRITE_REG(hw, IXGBE_RDLEN(reg_idx),
> ring->count * sizeof(union ixgbe_adv_rx_desc));
> + /* Force flushing of IXGBE_RDLEN to prevent MDD */
> + IXGBE_WRITE_FLUSH(hw);
> +
> IXGBE_WRITE_REG(hw, IXGBE_RDH(reg_idx), 0);
> IXGBE_WRITE_REG(hw, IXGBE_RDT(reg_idx), 0);
> ring->tail = adapter->io_addr + IXGBE_RDT(reg_idx);
> @@ -7641,6 +7644,21 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
> goto out_drop;
>
> goto xmit_fcoe;
> + } else if (protocol == htons(ETH_P_FIP)) {
> + /* A zero as offset will result in HW generating malicious event
> + * when trying to Tx a FIP frame. So, set a valid header length
> + * to prevent such an event.
> + */
> + if (!skb_network_offset(skb)) {
> + if (tx_flags & (IXGBE_TX_FLAGS_HW_VLAN |
> + IXGBE_TX_FLAGS_SW_VLAN))
> + skb_set_network_header(skb,
> + sizeof(struct ethhdr) +
> + sizeof(struct vlan_hdr));
> + else
> + skb_set_network_header(skb,
> + sizeof(struct ethhdr));
> + }
> }
>
I'm pretty sure this code is incorrect. The offset would only require
the vlan_hdr if to be included if the hardware was not offloading the
VLAN tag insertion.
Instead of setting the network header in the driver why not locate where
the FIP frame is being generated and set it there in the first place?
That way you can save anyone else who might be working with FCoE the
trouble of having to correct the layout of the frames?
- Alex
^ permalink raw reply [flat|nested] 3+ messages in thread* [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when FCoE+SRIOV are enabled
2015-12-08 16:58 ` Alexander Duyck
@ 2015-12-08 18:24 ` Parikh, Neerav
0 siblings, 0 replies; 3+ messages in thread
From: Parikh, Neerav @ 2015-12-08 18:24 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
> Sent: Tuesday, December 8, 2015 8:58 AM
> To: Parikh, Neerav; intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when
> FCoE+SRIOV are enabled
>
> On 12/07/2015 01:43 PM, Neerav Parikh wrote:
> > When FCoE is enabled with SR-IOV on the X550 NIC the hardware
> > generates MDD events.
> >
> > This patch fixes these by setting the expected values in the
> > Tx context descriptors for FCoE/FIP frames and adding a flush
> > after writing the RDLEN register.
> >
> > Signed-off-by: Neerav Parikh <neerav.parikh@intel.com>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 5 ++++-
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ++++++++++++++++++
> > drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
> > 3 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > index 5f98870..78ffef4 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> > @@ -517,6 +517,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
> > u32 vlan_macip_lens;
> > u32 fcoe_sof_eof = 0;
> > u32 mss_l4len_idx;
> > + u32 type_tucmd = IXGBE_ADVTXT_TUCMD_FCOE;
> > u8 sof, eof;
> >
> > if (skb_is_gso(skb) && (skb_shinfo(skb)->gso_type != SKB_GSO_FCOE)) {
> > @@ -593,6 +594,8 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
> > skb_shinfo(skb)->gso_size);
> > first->bytecount += (first->gso_segs - 1) * *hdr_len;
> > first->tx_flags |= IXGBE_TX_FLAGS_TSO;
> > + /* Hardware expects L4T to be RSV for FCoE TSO */
> > + type_tucmd |= IXGBE_ADVTXD_TUCMD_L4T_RSV;
> > }
> >
> > /* set flag indicating FCOE to ixgbe_tx_map call */
> > @@ -610,7 +613,7 @@ int ixgbe_fso(struct ixgbe_ring *tx_ring,
> >
> > /* write context desc */
> > ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fcoe_sof_eof,
> > - IXGBE_ADVTXT_TUCMD_FCOE, mss_l4len_idx);
> > + type_tucmd, mss_l4len_idx);
> >
> > return 0;
> > }
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index ebd4522..c31ed4c 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -3619,6 +3619,9 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter
> *adapter,
> > IXGBE_WRITE_REG(hw, IXGBE_RDBAH(reg_idx), (rdba >> 32));
> > IXGBE_WRITE_REG(hw, IXGBE_RDLEN(reg_idx),
> > ring->count * sizeof(union ixgbe_adv_rx_desc));
> > + /* Force flushing of IXGBE_RDLEN to prevent MDD */
> > + IXGBE_WRITE_FLUSH(hw);
> > +
> > IXGBE_WRITE_REG(hw, IXGBE_RDH(reg_idx), 0);
> > IXGBE_WRITE_REG(hw, IXGBE_RDT(reg_idx), 0);
> > ring->tail = adapter->io_addr + IXGBE_RDT(reg_idx);
> > @@ -7641,6 +7644,21 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff
> *skb,
> > goto out_drop;
> >
> > goto xmit_fcoe;
> > + } else if (protocol == htons(ETH_P_FIP)) {
> > + /* A zero as offset will result in HW generating malicious event
> > + * when trying to Tx a FIP frame. So, set a valid header length
> > + * to prevent such an event.
> > + */
> > + if (!skb_network_offset(skb)) {
> > + if (tx_flags & (IXGBE_TX_FLAGS_HW_VLAN |
> > + IXGBE_TX_FLAGS_SW_VLAN))
> > + skb_set_network_header(skb,
> > + sizeof(struct ethhdr) +
> > + sizeof(struct vlan_hdr));
> > + else
> > + skb_set_network_header(skb,
> > + sizeof(struct ethhdr));
> > + }
> > }
> >
>
> I'm pretty sure this code is incorrect. The offset would only require
> the vlan_hdr if to be included if the hardware was not offloading the
> VLAN tag insertion.
>
> Instead of setting the network header in the driver why not locate where
> the FIP frame is being generated and set it there in the first place?
> That way you can save anyone else who might be working with FCoE the
> trouble of having to correct the layout of the frames?
>
> - Alex
Thanks for reviewing this Alex; the only reason I added it in the driver was because
there may be multiple sources for the FIP frames (user space or FCoE stack) and
MDD from the ixgbe driver would show as some kind of error from the ixgbe driver
v/s the source that generated or created the packet.
I'll remove this portion from the patch for now and see if I can address the issue in the
FCoE stack where the FIP frames are generated as per your suggestion.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-08 18:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-07 21:43 [Intel-wired-lan] [PATCH] ixgbe: Fix MDD events generated when FCoE+SRIOV are enabled Neerav Parikh
2015-12-08 16:58 ` Alexander Duyck
2015-12-08 18:24 ` Parikh, Neerav
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.