* [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback
@ 2023-06-01 7:46 Magnus Karlsson
2023-06-01 9:53 ` Maciej Fijalkowski
2023-06-14 19:12 ` Brett Creeley
0 siblings, 2 replies; 4+ messages in thread
From: Magnus Karlsson @ 2023-06-01 7:46 UTC (permalink / raw)
To: magnus.karlsson, intel-wired-lan, anthony.l.nguyen,
maciej.fijalkowski, hao.ma
Cc: netdev
From: Hao Ma <hao.ma@intel.com>
Add support for NETIF_F_LOOPBACK. This feature can be set via: $
ethtool -K eth0 loopback <on|off>. This sets the MAC Tx->Rx loopback
used by selftests/bpf/xskxceiver.
Signed-off-by: Hao Ma <hao.ma@intel.com>
---
.../net/ethernet/intel/ixgbe/ixgbe_common.c | 4 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 73 +++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
3 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 878dd8dff528..b8998a56ad24 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -3337,7 +3337,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
if (link_up_wait_to_complete) {
for (i = 0; i < IXGBE_LINK_UP_TIME; i++) {
- if (links_reg & IXGBE_LINKS_UP) {
+ if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
*link_up = true;
break;
} else {
@@ -3347,7 +3347,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
links_reg = IXGBE_READ_REG(hw, IXGBE_LINKS);
}
} else {
- if (links_reg & IXGBE_LINKS_UP) {
+ if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
if (crosstalk_fix_active) {
/* Check the link state again after a delay
* to filter out spurious link up
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 5d83c887a3fc..70b34b7b5cb0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8864,6 +8864,57 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
return NETDEV_TX_OK;
}
+static int ixgbe_force_loopback(struct ixgbe_adapter *adapter, bool on)
+{ struct ixgbe_hw *hw = &adapter->hw;
+ u32 reg_data;
+
+ hw->loopback_on = on;
+ /* Setup MAC loopback */
+ reg_data = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+ if (on)
+ reg_data |= IXGBE_HLREG0_LPBK;
+ else
+ reg_data &= ~IXGBE_HLREG0_LPBK;
+ IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg_data);
+
+ reg_data = IXGBE_READ_REG(hw, IXGBE_FCTRL);
+ if (on)
+ reg_data |= IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE;
+ else
+ reg_data &= ~(IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE);
+ reg_data &= ~(IXGBE_FCTRL_BAM);
+ IXGBE_WRITE_REG(hw, IXGBE_FCTRL, reg_data);
+
+ /* X540 and X550 needs to set the MACC.FLU bit to force link up */
+ switch (adapter->hw.mac.type) {
+ case ixgbe_mac_X540:
+ case ixgbe_mac_X550:
+ case ixgbe_mac_X550EM_x:
+ case ixgbe_mac_x550em_a:
+ reg_data = IXGBE_READ_REG(hw, IXGBE_MACC);
+ if (on)
+ reg_data |= IXGBE_MACC_FLU;
+ else
+ reg_data &= ~IXGBE_MACC_FLU;
+ IXGBE_WRITE_REG(hw, IXGBE_MACC, reg_data);
+ break;
+ default:
+ if (hw->mac.orig_autoc) {
+ if (on)
+ reg_data = hw->mac.orig_autoc | IXGBE_AUTOC_FLU;
+ else
+ reg_data = hw->mac.orig_autoc & ~IXGBE_AUTOC_FLU;
+ IXGBE_WRITE_REG(hw, IXGBE_AUTOC, reg_data);
+ } else {
+ return 10;
+ }
+ }
+
+ IXGBE_WRITE_FLUSH(hw);
+
+ return 0;
+}
+
static netdev_tx_t __ixgbe_xmit_frame(struct sk_buff *skb,
struct net_device *netdev,
struct ixgbe_ring *ring)
@@ -9915,6 +9966,15 @@ static int ixgbe_set_features(struct net_device *netdev,
if (changed & NETIF_F_RXALL)
need_reset = true;
+ if (changed & NETIF_F_LOOPBACK) {
+ if (features & NETIF_F_LOOPBACK) {
+ ixgbe_force_loopback(adapter, true);
+ } else {
+ ixgbe_force_loopback(adapter, false);
+ need_reset = true;
+ }
+ }
+
netdev->features = features;
if ((changed & NETIF_F_HW_L2FW_DOFFLOAD) && adapter->num_rx_pools > 1)
@@ -10286,6 +10346,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
/* Wait until ndo_xsk_wakeup completes. */
synchronize_rcu();
err = ixgbe_setup_tc(dev, adapter->hw_tcs);
+ if (adapter->hw.loopback_on) {
+ u32 reg_data;
+
+ reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_HLREG0);
+ reg_data |= IXGBE_HLREG0_LPBK;
+ IXGBE_WRITE_REG(&adapter->hw, IXGBE_HLREG0, reg_data);
+
+ reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_MACC);
+ reg_data |= IXGBE_MACC_FLU;
+ IXGBE_WRITE_REG(&adapter->hw, IXGBE_MACC, reg_data);
+ }
if (err)
return -EINVAL;
@@ -10969,6 +11040,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (hw->mac.type >= ixgbe_mac_82599EB)
netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
+ netdev->features |= NETIF_F_LOOPBACK;
+
#ifdef CONFIG_IXGBE_IPSEC
#define IXGBE_ESP_FEATURES (NETIF_F_HW_ESP | \
NETIF_F_HW_ESP_TX_CSUM | \
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 2b00db92b08f..ca50ccd59b50 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3652,6 +3652,7 @@ struct ixgbe_hw {
bool allow_unsupported_sfp;
bool wol_enabled;
bool need_crosstalk_fix;
+ bool loopback_on;
};
struct ixgbe_info {
base-commit: 735c9ee9a374769b78c716de3c19a6c9440ede85
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback
2023-06-01 7:46 [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback Magnus Karlsson
@ 2023-06-01 9:53 ` Maciej Fijalkowski
2023-06-14 10:02 ` Ma, Hao
2023-06-14 19:12 ` Brett Creeley
1 sibling, 1 reply; 4+ messages in thread
From: Maciej Fijalkowski @ 2023-06-01 9:53 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, intel-wired-lan, anthony.l.nguyen, hao.ma,
netdev
On Thu, Jun 01, 2023 at 09:46:21AM +0200, Magnus Karlsson wrote:
> From: Hao Ma <hao.ma@intel.com>
>
> Add support for NETIF_F_LOOPBACK. This feature can be set via: $
> ethtool -K eth0 loopback <on|off>. This sets the MAC Tx->Rx loopback
> used by selftests/bpf/xskxceiver.
>
> Signed-off-by: Hao Ma <hao.ma@intel.com>
> ---
> .../net/ethernet/intel/ixgbe/ixgbe_common.c | 4 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 73 +++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
> 3 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> index 878dd8dff528..b8998a56ad24 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> @@ -3337,7 +3337,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
>
> if (link_up_wait_to_complete) {
> for (i = 0; i < IXGBE_LINK_UP_TIME; i++) {
> - if (links_reg & IXGBE_LINKS_UP) {
> + if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
> *link_up = true;
> break;
> } else {
> @@ -3347,7 +3347,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
> links_reg = IXGBE_READ_REG(hw, IXGBE_LINKS);
> }
> } else {
> - if (links_reg & IXGBE_LINKS_UP) {
> + if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
> if (crosstalk_fix_active) {
> /* Check the link state again after a delay
> * to filter out spurious link up
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 5d83c887a3fc..70b34b7b5cb0 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -8864,6 +8864,57 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
> return NETDEV_TX_OK;
> }
>
> +static int ixgbe_force_loopback(struct ixgbe_adapter *adapter, bool on)
> +{ struct ixgbe_hw *hw = &adapter->hw;
> + u32 reg_data;
> +
> + hw->loopback_on = on;
> + /* Setup MAC loopback */
> + reg_data = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> + if (on)
> + reg_data |= IXGBE_HLREG0_LPBK;
> + else
> + reg_data &= ~IXGBE_HLREG0_LPBK;
> + IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg_data);
> +
> + reg_data = IXGBE_READ_REG(hw, IXGBE_FCTRL);
> + if (on)
> + reg_data |= IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE;
> + else
> + reg_data &= ~(IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE);
> + reg_data &= ~(IXGBE_FCTRL_BAM);
> + IXGBE_WRITE_REG(hw, IXGBE_FCTRL, reg_data);
> +
> + /* X540 and X550 needs to set the MACC.FLU bit to force link up */
> + switch (adapter->hw.mac.type) {
> + case ixgbe_mac_X540:
> + case ixgbe_mac_X550:
> + case ixgbe_mac_X550EM_x:
> + case ixgbe_mac_x550em_a:
> + reg_data = IXGBE_READ_REG(hw, IXGBE_MACC);
> + if (on)
> + reg_data |= IXGBE_MACC_FLU;
> + else
> + reg_data &= ~IXGBE_MACC_FLU;
> + IXGBE_WRITE_REG(hw, IXGBE_MACC, reg_data);
> + break;
> + default:
> + if (hw->mac.orig_autoc) {
> + if (on)
> + reg_data = hw->mac.orig_autoc | IXGBE_AUTOC_FLU;
> + else
> + reg_data = hw->mac.orig_autoc & ~IXGBE_AUTOC_FLU;
> + IXGBE_WRITE_REG(hw, IXGBE_AUTOC, reg_data);
> + } else {
> + return 10;
> + }
> + }
> +
> + IXGBE_WRITE_FLUSH(hw);
> +
> + return 0;
> +}
> +
> static netdev_tx_t __ixgbe_xmit_frame(struct sk_buff *skb,
> struct net_device *netdev,
> struct ixgbe_ring *ring)
> @@ -9915,6 +9966,15 @@ static int ixgbe_set_features(struct net_device *netdev,
> if (changed & NETIF_F_RXALL)
> need_reset = true;
>
> + if (changed & NETIF_F_LOOPBACK) {
> + if (features & NETIF_F_LOOPBACK) {
> + ixgbe_force_loopback(adapter, true);
> + } else {
> + ixgbe_force_loopback(adapter, false);
> + need_reset = true;
> + }
on ice we just do:
if (changed & NETIF_F_LOOPBACK)
ret = ice_set_loopback(vsi, !!(features & NETIF_F_LOOPBACK));
don't you need to set need_reset to true as well when you are disabling
loopback in order to clear appropriate bits from registers?
> + }
> +
> netdev->features = features;
>
> if ((changed & NETIF_F_HW_L2FW_DOFFLOAD) && adapter->num_rx_pools > 1)
> @@ -10286,6 +10346,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> /* Wait until ndo_xsk_wakeup completes. */
> synchronize_rcu();
> err = ixgbe_setup_tc(dev, adapter->hw_tcs);
> + if (adapter->hw.loopback_on) {
> + u32 reg_data;
> +
> + reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_HLREG0);
> + reg_data |= IXGBE_HLREG0_LPBK;
> + IXGBE_WRITE_REG(&adapter->hw, IXGBE_HLREG0, reg_data);
> +
> + reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_MACC);
> + reg_data |= IXGBE_MACC_FLU;
> + IXGBE_WRITE_REG(&adapter->hw, IXGBE_MACC, reg_data);
can you explain why do you need to set loopback bits again after they were
set on ixgbe_force_loopback() ? are they not preserved after downing an
interface?
> + }
>
> if (err)
> return -EINVAL;
> @@ -10969,6 +11040,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (hw->mac.type >= ixgbe_mac_82599EB)
> netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
>
> + netdev->features |= NETIF_F_LOOPBACK;
> +
> #ifdef CONFIG_IXGBE_IPSEC
> #define IXGBE_ESP_FEATURES (NETIF_F_HW_ESP | \
> NETIF_F_HW_ESP_TX_CSUM | \
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 2b00db92b08f..ca50ccd59b50 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3652,6 +3652,7 @@ struct ixgbe_hw {
> bool allow_unsupported_sfp;
> bool wol_enabled;
> bool need_crosstalk_fix;
> + bool loopback_on;
please explain why are you introducing this flag
> };
>
> struct ixgbe_info {
>
> base-commit: 735c9ee9a374769b78c716de3c19a6c9440ede85
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback
2023-06-01 9:53 ` Maciej Fijalkowski
@ 2023-06-14 10:02 ` Ma, Hao
0 siblings, 0 replies; 4+ messages in thread
From: Ma, Hao @ 2023-06-14 10:02 UTC (permalink / raw)
To: Fijalkowski, Maciej, Magnus Karlsson
Cc: Karlsson, Magnus, intel-wired-lan@lists.osuosl.org,
Nguyen, Anthony L, netdev@vger.kernel.org
> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Sent: Thursday, June 1, 2023 5:54 PM
> To: Magnus Karlsson <magnus.karlsson@gmail.com>
> Cc: Karlsson, Magnus <magnus.karlsson@intel.com>; intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> Ma, Hao <hao.ma@intel.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback
>
> On Thu, Jun 01, 2023 at 09:46:21AM +0200, Magnus Karlsson wrote:
> > From: Hao Ma <hao.ma@intel.com>
> >
> > Add support for NETIF_F_LOOPBACK. This feature can be set via: $
> > ethtool -K eth0 loopback <on|off>. This sets the MAC Tx->Rx loopback
> > used by selftests/bpf/xskxceiver.
> >
> > Signed-off-by: Hao Ma <hao.ma@intel.com>
> > ---
> > .../net/ethernet/intel/ixgbe/ixgbe_common.c | 4 +-
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 73
> > +++++++++++++++++++ drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |
> > 1 +
> > 3 files changed, 76 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > index 878dd8dff528..b8998a56ad24 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > @@ -3337,7 +3337,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw
> > *hw, ixgbe_link_speed *speed,
> >
> > if (link_up_wait_to_complete) {
> > for (i = 0; i < IXGBE_LINK_UP_TIME; i++) {
> > - if (links_reg & IXGBE_LINKS_UP) {
> > + if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
> > *link_up = true;
> > break;
> > } else {
> > @@ -3347,7 +3347,7 @@ s32 ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
> > links_reg = IXGBE_READ_REG(hw, IXGBE_LINKS);
> > }
> > } else {
> > - if (links_reg & IXGBE_LINKS_UP) {
> > + if (links_reg & IXGBE_LINKS_UP || hw->loopback_on) {
> > if (crosstalk_fix_active) {
> > /* Check the link state again after a delay
> > * to filter out spurious link up diff --git
> > a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 5d83c887a3fc..70b34b7b5cb0 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -8864,6 +8864,57 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
> > return NETDEV_TX_OK;
> > }
> >
> > +static int ixgbe_force_loopback(struct ixgbe_adapter *adapter, bool on)
> > +{ struct ixgbe_hw *hw = &adapter->hw;
> > + u32 reg_data;
> > +
> > + hw->loopback_on = on;
> > + /* Setup MAC loopback */
> > + reg_data = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> > + if (on)
> > + reg_data |= IXGBE_HLREG0_LPBK;
> > + else
> > + reg_data &= ~IXGBE_HLREG0_LPBK;
> > + IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg_data);
> > +
> > + reg_data = IXGBE_READ_REG(hw, IXGBE_FCTRL);
> > + if (on)
> > + reg_data |= IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE;
> > + else
> > + reg_data &= ~(IXGBE_FCTRL_SBP | IXGBE_FCTRL_MPE);
> > + reg_data &= ~(IXGBE_FCTRL_BAM);
> > + IXGBE_WRITE_REG(hw, IXGBE_FCTRL, reg_data);
> > +
> > + /* X540 and X550 needs to set the MACC.FLU bit to force link up */
> > + switch (adapter->hw.mac.type) {
> > + case ixgbe_mac_X540:
> > + case ixgbe_mac_X550:
> > + case ixgbe_mac_X550EM_x:
> > + case ixgbe_mac_x550em_a:
> > + reg_data = IXGBE_READ_REG(hw, IXGBE_MACC);
> > + if (on)
> > + reg_data |= IXGBE_MACC_FLU;
> > + else
> > + reg_data &= ~IXGBE_MACC_FLU;
> > + IXGBE_WRITE_REG(hw, IXGBE_MACC, reg_data);
> > + break;
> > + default:
> > + if (hw->mac.orig_autoc) {
> > + if (on)
> > + reg_data = hw->mac.orig_autoc | IXGBE_AUTOC_FLU;
> > + else
> > + reg_data = hw->mac.orig_autoc & ~IXGBE_AUTOC_FLU;
> > + IXGBE_WRITE_REG(hw, IXGBE_AUTOC, reg_data);
> > + } else {
> > + return 10;
> > + }
> > + }
> > +
> > + IXGBE_WRITE_FLUSH(hw);
> > +
> > + return 0;
> > +}
> > +
> > static netdev_tx_t __ixgbe_xmit_frame(struct sk_buff *skb,
> > struct net_device *netdev,
> > struct ixgbe_ring *ring)
> > @@ -9915,6 +9966,15 @@ static int ixgbe_set_features(struct net_device *netdev,
> > if (changed & NETIF_F_RXALL)
> > need_reset = true;
> >
> > + if (changed & NETIF_F_LOOPBACK) {
> > + if (features & NETIF_F_LOOPBACK) {
> > + ixgbe_force_loopback(adapter, true);
> > + } else {
> > + ixgbe_force_loopback(adapter, false);
> > + need_reset = true;
> > + }
>
> on ice we just do:
>
> if (changed & NETIF_F_LOOPBACK)
> ret = ice_set_loopback(vsi, !!(features & NETIF_F_LOOPBACK));
>
> don't you need to set need_reset to true as well when you are disabling loopback in order to clear appropriate bits from registers?
>
ixgbe driver loopback enablement can interfere with NIC normal function configuration, so after test, a reset is needed and reconfigure NIC back to normal function.
> > + }
> > +
> > netdev->features = features;
> >
> > if ((changed & NETIF_F_HW_L2FW_DOFFLOAD) && adapter->num_rx_pools >
> > 1) @@ -10286,6 +10346,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
> > /* Wait until ndo_xsk_wakeup completes. */
> > synchronize_rcu();
> > err = ixgbe_setup_tc(dev, adapter->hw_tcs);
> > + if (adapter->hw.loopback_on) {
> > + u32 reg_data;
> > +
> > + reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_HLREG0);
> > + reg_data |= IXGBE_HLREG0_LPBK;
> > + IXGBE_WRITE_REG(&adapter->hw, IXGBE_HLREG0, reg_data);
> > +
> > + reg_data = IXGBE_READ_REG(&adapter->hw, IXGBE_MACC);
> > + reg_data |= IXGBE_MACC_FLU;
> > + IXGBE_WRITE_REG(&adapter->hw, IXGBE_MACC, reg_data);
>
> can you explain why do you need to set loopback bits again after they were set on ixgbe_force_loopback() ? are they not preserved after
> downing an interface?
>
In ixgbe NIC XDP initialization process, the corresponding register will be reset.
> > + }
> >
> > if (err)
> > return -EINVAL;
> > @@ -10969,6 +11040,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > if (hw->mac.type >= ixgbe_mac_82599EB)
> > netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
> >
> > + netdev->features |= NETIF_F_LOOPBACK;
> > +
> > #ifdef CONFIG_IXGBE_IPSEC
> > #define IXGBE_ESP_FEATURES (NETIF_F_HW_ESP | \
> > NETIF_F_HW_ESP_TX_CSUM | \
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > index 2b00db92b08f..ca50ccd59b50 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > @@ -3652,6 +3652,7 @@ struct ixgbe_hw {
> > bool allow_unsupported_sfp;
> > bool wol_enabled;
> > bool need_crosstalk_fix;
> > + bool loopback_on;
>
> please explain why are you introducing this flag
>
Please refer to the previous comment.
> > };
> >
> > struct ixgbe_info {
> >
> > base-commit: 735c9ee9a374769b78c716de3c19a6c9440ede85
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback
2023-06-01 7:46 [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback Magnus Karlsson
2023-06-01 9:53 ` Maciej Fijalkowski
@ 2023-06-14 19:12 ` Brett Creeley
1 sibling, 0 replies; 4+ messages in thread
From: Brett Creeley @ 2023-06-14 19:12 UTC (permalink / raw)
To: Magnus Karlsson, magnus.karlsson, intel-wired-lan,
anthony.l.nguyen, maciej.fijalkowski, hao.ma
Cc: netdev
On 6/1/2023 12:46 AM, Magnus Karlsson wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> From: Hao Ma <hao.ma@intel.com>
>
> Add support for NETIF_F_LOOPBACK. This feature can be set via: $
> ethtool -K eth0 loopback <on|off>. This sets the MAC Tx->Rx loopback
> used by selftests/bpf/xskxceiver.
>
> Signed-off-by: Hao Ma <hao.ma@intel.com>
> ---
> .../net/ethernet/intel/ixgbe/ixgbe_common.c | 4 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 73 +++++++++++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
> 3 files changed, 76 insertions(+), 2 deletions(-)
>
[...]
> @@ -10969,6 +11040,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (hw->mac.type >= ixgbe_mac_82599EB)
> netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
>
> + netdev->features |= NETIF_F_LOOPBACK;
This means NETIF_F_LOOPBACK is enabled by default. Was that the
intention? It seems like NETIF_F_LOOPBACK should only be set for
netdev->hw_features on probe. I didn't test this, so it would be worth
double this patch functions as expected with this change.
> +
> #ifdef CONFIG_IXGBE_IPSEC
> #define IXGBE_ESP_FEATURES (NETIF_F_HW_ESP | \
> NETIF_F_HW_ESP_TX_CSUM | \
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 2b00db92b08f..ca50ccd59b50 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3652,6 +3652,7 @@ struct ixgbe_hw {
> bool allow_unsupported_sfp;
> bool wol_enabled;
> bool need_crosstalk_fix;
> + bool loopback_on;
> };
>
> struct ixgbe_info {
>
> base-commit: 735c9ee9a374769b78c716de3c19a6c9440ede85
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-14 19:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-01 7:46 [PATCH iwl-next] ixgbe: allow toggling loopback mode via ndo_set_features callback Magnus Karlsson
2023-06-01 9:53 ` Maciej Fijalkowski
2023-06-14 10:02 ` Ma, Hao
2023-06-14 19:12 ` Brett Creeley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).