fm10k: remove is_slot_appropriate
Subsystems (2)
PATCH 1/10 fm10k: remove is_slot_appropriate
This function is no longer used now that we have updated fm10k_slot_warn functionality.
--- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 14 -------------- drivers/net/ethernet/intel/fm10k/fm10k_type.h | 1 - drivers/net/ethernet/intel/fm10k/fm10k_vf.c | 14 -------------- 3 files changed, 29 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0 -14
@@ -185,19 +185,6 @@ static s32 fm10k_init_hw_pf(struct fm10k_hw *hw) }/** -* fm10k_is_slot_appropriate_pf - Indicate appropriate slot for this SKU -* @hw: pointer to hardware structure -* -* Looks at the PCIe bus info to confirm whether or not this slot can support -* the necessary bandwidth for this device. -**/ -static bool fm10k_is_slot_appropriate_pf (struct fm10k_hw *hw) -{ -return (hw->bus.speed == hw->bus_caps.speed) && - (hw->bus.width == hw->bus_caps.width); -} - -/** * fm10k_update_vlan_pf - Update status of VLAN ID in VLAN filter table * @hw: pointer to hardware structure * @vid: VLAN ID to add to table
@@ -1849,7 +1836,6 @@ static struct fm10k_mac_ops mac_ops_pf = { .init_hw = &fm10k_init_hw_pf, .start_hw = &fm10k_start_hw_generic, .stop_hw = &fm10k_stop_hw_generic, - .is_slot_appropriate = &fm10k_is_slot_appropriate_pf, .update_vlan = &fm10k_update_vlan_pf, .read_mac_addr = &fm10k_read_mac_addr_pf, .update_uc_addr = &fm10k_update_uc_addr_pf,
drivers/net/ethernet/intel/fm10k/fm10k_type.h +0 -1
@@ -521,7 +521,6 @@ struct fm10k_mac_ops {s32 (*stop_hw)(struct fm10k_hw *);s32 (*get_bus_info)(struct fm10k_hw *);s32 (*get_host_state)(struct fm10k_hw *,bool *); -bool (*is_slot_appropriate)(struct fm10k_hw *);s32 (*update_vlan)(struct fm10k_hw *,u32 ,u8 ,bool );s32 (*read_mac_addr)(struct fm10k_hw *); s32 (*update_uc_addr)(struct fm10k_hw *, u16, const u8 *,
drivers/net/ethernet/intel/fm10k/fm10k_vf.c +0 -14
@@ -131,19 +131,6 @@ static s32 fm10k_init_hw_vf(struct fm10k_hw *hw)return 0 ; } -/** -* fm10k_is_slot_appropriate_vf - Indicate appropriate slot for this SKU -* @hw: pointer to hardware structure -* -* Looks at the PCIe bus info to confirm whether or not this slot can support -* the necessary bandwidth for this device. Since the VF has no control over -* the "slot" it is in, always indicate that the slot is appropriate. -**/ -static bool fm10k_is_slot_appropriate_vf (struct fm10k_hw *hw) -{ -return true; -} -/* This structure defines the attibutes to be parsed below */ const struct fm10k_tlv_attr fm10k_mac_vlan_msg_attr[] = {FM10K_TLV_ATTR_U32 (FM10K_MAC_VLAN_MSG_VLAN ),
@@ -552,7 +539,6 @@ static struct fm10k_mac_ops mac_ops_vf = { .init_hw = &fm10k_init_hw_vf, .start_hw = &fm10k_start_hw_generic, .stop_hw = &fm10k_stop_hw_vf, - .is_slot_appropriate = &fm10k_is_slot_appropriate_vf, .update_vlan = &fm10k_update_vlan_vf, .read_mac_addr = &fm10k_read_mac_addr_vf, .update_uc_addr = &fm10k_update_uc_addr_vf,
Signature · 1 line
2.4.3
PATCH 2/10 fm10k: allow creation of vlan interfaces even while down
We re-sync upon going up, so there is little reason to worry about not syncing immediately with switch. This prevents an error that occurs if you add a vlan interface while down.
--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +6 -0
@@ -779,6 +779,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set && vid == hw->mac.default_vid)return 0 ; +/* Do not throw an error if the interface is down. We will sync once +* we come up +*/ +if (test_bit (__FM10K_DOWN, &interface->state)) +return 0 ; +fm10k_mbx_lock (interface);/* only need to update the VLAN if not in promiscuous mode */
Signature · 1 line
2.4.3
PATCH 3/10 fm10k: don't store sw_vid at reset
In reply to Jacob KellerIf we store the sw_vid at reset of PF, then we accidentally prevent the VF from receiving the message to update its default VID. This only occurs if the VF is created before the PF has come up, which is the standard way of creating VFs when using the module parameter.
This fixes an issue where we request the incorrect MAC/VLAN combinations, and prevents us from accidentally reporting some frames as vlan tagged.
--- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 --- 1 file changed, 3 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_iov.c +0 -3
@@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev) hw->iov.ops.set_lport (hw, vf_info, i,FM10K_VF_FLAG_MULTI_CAPABLE ); -/* assign our default vid to the VF following reset */ - vf_info->sw_vid = hw->mac.default_vid; -/* mailbox is disconnected so we don't send a message */ hw->iov.ops.assign_default_mac_vlan (hw, vf_info);
Signature · 1 line
2.4.3
PATCH 3/10 fm10k: don't store sw_vid at reset
Quoted 4 lines · Jacob Keller
If we store the
sw_vidat reset of PF, then we accidentally prevent the VF from receiving the message to update its default VID. This only occurs if the VF is created before the PF has come up, which is the standard way of creating VFs when using the module parameter.
The upstream driver doesn't have a module parameter for VFs. It uses sysfs. It isn't clear but can this issue occur if you use the sysfs?
Quoted 14 lines
This fixes an issue where we request the incorrect MAC/VLAN combinations, and prevents us from accidentally reporting some frames as vlan tagged.
Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 --- 1 file changed, 3 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_iov.c +0
-0
drivers/net/ethernet/intel/fm10k/fm10k_iov.c int fm10k_iov_resume(struct pci_dev *pdev) · line 233
Quoted 9 lines
drivers/net/ethernet/intel/fm10k/fm10k_iov.c +0
-3@@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev) hw->iov.ops.set_lport (hw, vf_info, i,FM10K_VF_FLAG_MULTI_CAPABLE ); -/* assign our default vid to the VF following reset */ - vf_info->sw_vid = hw->mac.default_vid; -/* mailbox is disconnected so we don't send a message */ hw->iov.ops.assign_default_mac_vlan (hw, vf_info);
The patch itself looks fine. I assume the Switch API will send a message to force us onto the correct VID if there is one already reserved for this VF.
I probably shouldn't have used the PF VLAN ID for the VFs anyway. Just letting the Switch API bounce a message through is a much better way to go.
Keller
PATCH 3/10 fm10k: don't store sw_vid at reset
Quoted 11 lines · Alexander Duyck
Quoted 5 lines · Jacob Keller
If we store the
sw_vidat reset of PF, then we accidentally prevent the
VF from receiving the message to update its default VID. This only occurs if the VF is created before the PF has come up, which is the standard way of creating VFs when using the module parameter.The upstream driver doesn't have a module parameter for VFs. It uses
sysfs. It isn't clear but can this issue occur if you use the sysfs?
Yes, I can re-work the comment. It can happen if you instantiate VFs before you bring up the PF such as:
modprobe fm10k
echo N > /sys/class/.../sriov_numvfs
ip link set DEVICE up
If you do that in this order VFs will have the issue.
Quoted 33 lines
Quoted 25 lines
This fixes an issue where we request the incorrect MAC/VLAN combinations, and prevents us from accidentally reporting some frames as
vlan tagged.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 --- 1 file changed, 3 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_iov.c +0
-1@@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev) hw->iov.ops.set_lport (hw, vf_info, i,FM10K_VF_FLAG_MULTI_CAPABLE ); -/* assign our default vid to the VF following reset */
drivers/net/ethernet/intel/fm10k/fm10k_iov.c +0
-2- vf_info->sw_vid = hw->mac.default_vid; -/* mailbox is disconnected so we don't send a Quoted 3 lines
message */
hw->iov.ops.assign_default_mac_vlan(hw, vf_info);The patch itself looks fine. I assume the Switch API will send a message to force us onto the correct VID if there is one already reserved for this VF.
Quoted 4 lines
I probably shouldn't have used the PF VLAN ID for the VFs anyway. Just
letting the Switch API bounce a message through is a much better way to go.
Yes, the switch sends a message which we receive on coming up.
Regards,
Jake
PATCH 4/10 fm10k: TRIVIAL fix up ordering of __always_unused and style
In reply to Jacob KellerFix some style issues in debugfs code, and correct ordering of void and __always_unused. Technically, the order does not matter, but preferred style is to put the macro between the type and name.
--- drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c +3 -2
@@ -37,7 +37,8 @@ static void *fm10k_dbg_desc_seq_start(struct seq_file *s, loff_t *pos) }static void *fm10k_dbg_desc_seq_next (struct seq_file *s, -void __always_unused *v,loff_t *pos) +void __always_unused *v, +loff_t *pos) {struct fm10k_ring *ring = s->private;
@@ -45,7 +46,7 @@ static void *fm10k_dbg_desc_seq_next(struct seq_file *s, }static void fm10k_dbg_desc_seq_stop (struct seq_file __always_unused *s, -__always_unused void *v) +void __always_unused *v) {/* Do nothing. */ }
Signature · 1 line
2.4.3
PATCH 5/10 fm10k: add support for extra debug statistics
In reply to Jacob KellerAdd a private ethtool flag to enable display of these statistics, which are generally less useful. However, sometimes it can be useful for debugging purposes. The most useful portion is the ability to see what the PF thinks the VF mailboxes look like.
--- drivers/net/ethernet/intel/fm10k/fm10k.h | 19 ++ drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 224 +++++++++++++++++++---- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 5 +- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 15 ++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 18 ++ drivers/net/ethernet/intel/fm10k/fm10k_type.h | 6 + 6 files changed, 246 insertions(+), 41 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k.h +19 -0
@@ -101,12 +101,19 @@ struct fm10k_tx_queue_stats {u64 csum_err;u64 tx_busy;u64 tx_done_old; +u64 csum_good; };struct fm10k_rx_queue_stats {u64 alloc_failed;u64 csum_err;u64 errors; +u64 csum_good; +u64 switch_errors; +u64 drops; +u64 pp_errors; +u64 link_errors; +u64 length_errors; };struct fm10k_ring {
@@ -251,6 +258,7 @@ struct fm10k_intfc {#define FM10K_FLAG_RSS_FIELD_IPV6_UDP (u32)(1 << 2)#define FM10K_FLAG_RX_TS_ENABLED (u32)(1 << 3)#define FM10K_FLAG_SWPRI_CONFIG (u32)(1 << 4) +#define FM10K_FLAG_DEBUG_STATS (u32)(1 << 5)int xcast_mode;/* Tx fast path data */
@@ -277,6 +285,17 @@ struct fm10k_intfc {u64 rx_drops_nic;u64 rx_overrun_pf;u64 rx_overrun_vf; + +/* Debug Statistics */ +u64 hw_sm_mbx_full; +u64 hw_csum_tx_good; +u64 hw_csum_rx_good; +u64 rx_switch_errors; +u64 rx_drops; +u64 rx_pp_errors; +u64 rx_link_errors; +u64 rx_length_errors; +u32 tx_timeout_count;/* RX */
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c +184 -40
307 lines not on this page · raw message
drivers/net/ethernet/intel/fm10k/fm10k_iov.c +4 -1
@@ -137,8 +137,11 @@ process_mbx: }/* guarantee we have free space in the SM mailbox */ -if (!hw->mbx.ops.tx_ready (&hw->mbx,FM10K_VFMBX_MSG_MTU )) +if (!hw->mbx.ops.tx_ready (&hw->mbx,FM10K_VFMBX_MSG_MTU )) { +/* keep track of how many times this occurs */ + interface->hw_sm_mbx_full++;break ; + }/* cleanup mailbox and process received messages */ mbx->ops.process (hw, mbx);
drivers/net/ethernet/intel/fm10k/fm10k_main.c +15 -0
@@ -398,6 +398,8 @@ static inline void fm10k_rx_checksum(struct fm10k_ring *ring,return ; skb->ip_summed =CHECKSUM_UNNECESSARY ; + + ring->rx_stats.csum_good++; }#define FM10K_RSS_L4_TYPES_MASK \
@@ -553,6 +555,18 @@ static bool fm10k_cleanup_headers(struct fm10k_ring *rx_ring, {if (unlikely ((fm10k_test_staterr (rx_desc,FM10K_RXD_STATUS_RXE )))) { +#define FM10K_TEST_RXD_BIT (rxd, bit) \ + ((rxd)->w.csum_err & cpu_to_le16(bit)) +if (FM10K_TEST_RXD_BIT (rx_desc,FM10K_RXD_ERR_SWITCH_ERROR )) + rx_ring->rx_stats.switch_errors++; +if (FM10K_TEST_RXD_BIT (rx_desc,FM10K_RXD_ERR_NO_DESCRIPTOR )) + rx_ring->rx_stats.drops++; +if (FM10K_TEST_RXD_BIT (rx_desc,FM10K_RXD_ERR_PP_ERROR )) + rx_ring->rx_stats.pp_errors++; +if (FM10K_TEST_RXD_BIT (rx_desc,FM10K_RXD_ERR_SWITCH_READY )) + rx_ring->rx_stats.link_errors++; +if (FM10K_TEST_RXD_BIT (rx_desc,FM10K_RXD_ERR_TOO_BIG )) + rx_ring->rx_stats.length_errors++;dev_kfree_skb_any (skb); rx_ring->rx_stats.errors++;return true;
@@ -878,6 +892,7 @@ static void fm10k_tx_csum(struct fm10k_ring *tx_ring,/* update TX checksum flag */ first->tx_flags |=FM10K_TX_FLAGS_CSUM ; + tx_ring->tx_stats.csum_good++; no_csum:/* populate Tx descriptor header size and mss */
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +18 -0
@@ -328,6 +328,9 @@ void fm10k_update_stats(struct fm10k_intfc *interface) {struct net_device_stats *net_stats = &interface->netdev->stats;struct fm10k_hw *hw = &interface->hw; +u64 hw_csum_tx_good =0 , hw_csum_rx_good =0 , rx_length_errors =0 ; +u64 rx_switch_errors =0 , rx_drops =0 , rx_pp_errors =0 ; +u64 rx_link_errors =0 ;u64 rx_errors =0 , rx_csum_errors =0 , tx_csum_errors =0 ;u64 restart_queue =0 , tx_busy =0 , alloc_failed =0 ;u64 rx_bytes_nic =0 , rx_pkts_nic =0 , rx_drops_nic =0 ;
@@ -347,6 +350,7 @@ void fm10k_update_stats(struct fm10k_intfc *interface) tx_csum_errors += tx_ring->tx_stats.csum_err; bytes += tx_ring->stats.bytes; pkts += tx_ring->stats.packets; + hw_csum_tx_good += tx_ring->tx_stats.csum_good; } interface->restart_queue = restart_queue;
@@ -354,6 +358,8 @@ void fm10k_update_stats(struct fm10k_intfc *interface) net_stats->tx_bytes = bytes; net_stats->tx_packets = pkts; interface->tx_csum_errors = tx_csum_errors; + interface->hw_csum_tx_good = hw_csum_tx_good; +/* gather some stats to the interface struct that are per queue */ for (bytes =0 , pkts =0 , i =0 ; i < interface->num_rx_queues; i++) {struct fm10k_ring *rx_ring = interface->rx_ring[i];
@@ -363,12 +369,24 @@ void fm10k_update_stats(struct fm10k_intfc *interface) alloc_failed += rx_ring->rx_stats.alloc_failed; rx_csum_errors += rx_ring->rx_stats.csum_err; rx_errors += rx_ring->rx_stats.errors; + hw_csum_rx_good += rx_ring->rx_stats.csum_good; + rx_switch_errors += rx_ring->rx_stats.switch_errors; + rx_drops += rx_ring->rx_stats.drops; + rx_pp_errors += rx_ring->rx_stats.pp_errors; + rx_link_errors += rx_ring->rx_stats.link_errors; + rx_length_errors += rx_ring->rx_stats.length_errors; } net_stats->rx_bytes = bytes; net_stats->rx_packets = pkts; interface->alloc_failed = alloc_failed; interface->rx_csum_errors = rx_csum_errors; + interface->hw_csum_rx_good = hw_csum_rx_good; + interface->rx_switch_errors = rx_switch_errors; + interface->rx_drops = rx_drops; + interface->rx_pp_errors = rx_pp_errors; + interface->rx_link_errors = rx_link_errors; + interface->rx_length_errors = rx_length_errors; hw->mac.ops.update_hw_stats (hw, &interface->stats);
drivers/net/ethernet/intel/fm10k/fm10k_type.h +6 -0
@@ -762,6 +762,12 @@ enum fm10k_rxdesc_xc {#define FM10K_RXD_STATUS_L4E 0x4000/* L4 csum error */ #define FM10K_RXD_STATUS_IPE 0x8000/* IPv4 csum error */ +#define FM10K_RXD_ERR_SWITCH_ERROR 0x0001/* Switch found bad packet */ +#define FM10K_RXD_ERR_NO_DESCRIPTOR 0x0002/* No descriptor available */ +#define FM10K_RXD_ERR_PP_ERROR 0x0004/* RAM error during processing */ +#define FM10K_RXD_ERR_SWITCH_READY 0x0008/* Link transition mid-packet */ +#define FM10K_RXD_ERR_TOO_BIG 0x0010/* Pkt too big for single buf */ +struct fm10k_ftag {__be16 swpri_type_user;__be16 vlan;
Signature · 1 line
2.4.3
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
In reply to Jacob KellerThis patch ensures that vlan traffic on the default vid will go to the corresponding vlan device if it exists. To do this, mask the rx_ring vid if we have an active vlan on that vid.
--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +12 -0
@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)struct fm10k_intfc *interface =netdev_priv (netdev);struct fm10k_hw *hw = &interface->hw;s32 err; +int i;/* updates do not apply to VLAN 0 */ if (!vid)
@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set)clear_bit (vid, interface->active_vlans); +/* disable the default vid on ring if we have an active vlan */ +for (i =0 ; i < interface->num_rx_queues; i++) { +struct fm10k_ring *rx_ring = interface->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and MAC tables */ if (!set && vid == hw->mac.default_vid)return 0 ;
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4 -0
@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid; +/* if we have an active VLAN, disable default vid */ +if (test_bit (hw->mac.default_vid, interface->active_vlans)) + ring->vid |=FM10K_VLAN_CLEAR ; +/* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx +NON_Q_VECTORS (hw);
Signature · 1 line
2.4.3
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
Quoted 46 lines · Jacob Keller
This patch ensures that vlan traffic on the default vid will go to the corresponding vlan device if it exists. To do this, mask the
rx_ringvid if we have an active vlan on that vid.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +12
-0@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)struct fm10k_intfc *interface =netdev_priv (netdev);struct fm10k_hw *hw = &interface->hw;s32 err; +int i;/* updates do not apply to VLAN 0 */ if (!vid)@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set)clear_bit (vid, interface->active_vlans); +/* disable the default vid on ring if we have an active vlan */ +for (i =0 ; i < interface->num_rx_queues; i++) { +struct fm10k_ring *rx_ring = interface->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and MAC tables */ if (!set && vid == hw->mac.default_vid)return 0 ;drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_configure_rx_ring(struct fm10k_intfc *interface, · line 703
Quoted 10 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-0@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid; +/* if we have an active VLAN, disable default vid */ +if (test_bit (hw->mac.default_vid, interface->active_vlans)) + ring->vid |=FM10K_VLAN_CLEAR ; +/* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx +NON_Q_VECTORS (hw);
This doesn't quite work since the rx_ring->vid is doing a full 16b comparison against a value that includes the QOS value.
I just realized that is a bug in the fm10k_process_skb_fields function.
You probably need to update it so that code does something more like
if (rx_desc->w.vlan) {
u16 vid = le16_to_cpu(rx_desc->w.vlan);
if ((vid & VLAN_VID_MASK) != rx_ring->vid)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
else if (vid & VLAN_PRIO_MASK)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
vid & VLAN_PRIO_MASK);
} Vick
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
Quoted 1 line · Alexander Duyck
On 06/19/2015 04:37 PM, Jacob Keller wrote:
[...]
Quoted 25 lines
Quoted 16 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-0@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid; +/* if we have an active VLAN, disable default vid */ +if (test_bit (hw->mac.default_vid, interface->active_vlans)) + ring->vid |=FM10K_VLAN_CLEAR ; +/* Map interrupt */ if (ring->q_vector) {Quoted 2 lines
rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);This doesn't quite work since the
rx_ring->vidis doing a full 16b comparison against a value that includes the QOS value.I just realized that is a bug in the
fm10k_process_skb_fieldsfunction.
You probably need to update it so that code does something more like
Quoted 9 lines
if (rx_desc->w.vlan) {
u16 vid = le16_to_cpu(rx_desc->w.vlan);
if ((vid & VLAN_VID_MASK) != rx_ring->vid)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
else if (vid & VLAN_PRIO_MASK)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
vid & VLAN_PRIO_MASK);
}Agreed. Coincidentally, this bit us last week and we're working internally to resolve it now. Your solution looks good to me.
Keller
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
In reply to Alexander DuyckQuoted 59 lines · Alexander Duyck
Quoted 58 lines · Jacob Keller
This patch ensures that vlan traffic on the default vid will go to the
corresponding vlan device if it exists. To do this, mask therx_ringvid
if we have an active vlan on that vid.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)struct fm10k_intfc *interface =netdev_priv (netdev);struct fm10k_hw *hw = &interface->hw;s32 err; +int i;/* updates do not apply to VLAN 0 */ if (!vid)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set)clear_bit (vid, interface->active_vlans); +/* disable the default vid on ring if we have an active vlan */
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +10
-1+for (i =0 ; i < interface->num_rx_queues; i++) { + struct fm10k_ring *rx_ring = interface ->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and MAC tables */
if (!set && vid ==hw->mac.default_vid)
return 0;drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid;
Quoted 13 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_configure_rx_ring(struct fm10k_intfc *interface, · line 703
Quoted 9 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-1+/* if we have an active VLAN, disable default vid */ + if (test_bit(hw->mac.default_vid, interface ->active_vlans)) + ring->vid |= FM10K_VLAN_CLEAR; + /* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);This doesn't quite work since the
rx_ring->vidis doing a full 16b comparison against a value that includes the QOS value.
So we have to mask the QOS value?
Quoted 5 lines
I just realized that is a bug in the
fm10k_process_skb_fieldsfunction.
You probably need to update it so that code does something more
like
Quoted 10 lines
if (rx_desc->w.vlan) {
u16 vid = le16_to_cpu(rx_desc->w.vlan);
if ((vid & VLAN_VID_MASK) != rx_ring->vid)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
vid);
else if (vid & VLAN_PRIO_MASK)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
vid & VLAN_PRIO_MASK);
} I'll take a look at that.
Regards,
Jake
Keller
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
In reply to Alexander DuyckQuoted 59 lines · Alexander Duyck
Quoted 58 lines · Jacob Keller
This patch ensures that vlan traffic on the default vid will go to the
corresponding vlan device if it exists. To do this, mask therx_ringvid
if we have an active vlan on that vid.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)struct fm10k_intfc *interface =netdev_priv (netdev);struct fm10k_hw *hw = &interface->hw;s32 err; +int i;/* updates do not apply to VLAN 0 */ if (!vid)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set)clear_bit (vid, interface->active_vlans); +/* disable the default vid on ring if we have an active vlan */
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +10
-1+for (i =0 ; i < interface->num_rx_queues; i++) { + struct fm10k_ring *rx_ring = interface ->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and MAC tables */
if (!set && vid ==hw->mac.default_vid)
return 0;drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid;
Quoted 13 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_configure_rx_ring(struct fm10k_intfc *interface, · line 703
Quoted 9 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-1+/* if we have an active VLAN, disable default vid */ + if (test_bit(hw->mac.default_vid, interface ->active_vlans)) + ring->vid |= FM10K_VLAN_CLEAR; + /* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);This doesn't quite work since the
rx_ring->vidis doing a full 16b comparison against a value that includes the QOS value.
To clarify, this should work as long as we fix the process_skb_fields?
Regards,
Jake
Quoted 15 lines
I just realized that is a bug in the
fm10k_process_skb_fieldsfunction.
You probably need to update it so that code does something more
likeif (rx_desc->w.vlan) { u16 vid = le16_to_cpu(rx_desc->w.vlan); if ((vid & VLAN_VID_MASK) != rx_ring->vid) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid); else if (vid & VLAN_PRIO_MASK) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid & VLAN_PRIO_MASK); }
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
Quoted 72 lines · Jacob E
Quoted 59 lines · Alexander Duyck
Quoted 58 lines · Jacob Keller
This patch ensures that vlan traffic on the default vid will go to the
corresponding vlan device if it exists. To do this, mask therx_ringvid
if we have an active vlan on that vid.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)struct fm10k_intfc *interface =netdev_priv (netdev);struct fm10k_hw *hw = &interface->hw;s32 err; +int i;/* updates do not apply to VLAN 0 */ if (!vid)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)if (!set)clear_bit (vid, interface->active_vlans); +/* disable the default vid on ring if we have an active vlan */
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +10
-1+for (i =0 ; i < interface->num_rx_queues; i++) { + struct fm10k_ring *rx_ring = interface ->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and MAC tables */
if (!set && vid ==hw->mac.default_vid)
return 0;drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid;Quoted 12 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_configure_rx_ring(struct fm10k_intfc *interface, · line 703
Quoted 9 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-1+/* if we have an active VLAN, disable default vid */ + if (test_bit(hw->mac.default_vid, interface ->active_vlans)) + ring->vid |= FM10K_VLAN_CLEAR; + /* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);This doesn't quite work since the
rx_ring->vidis doing a full 16b comparison against a value that includes the QOS value.
Quoted 1 line
To clarify, this should work as long as we fix the
process_skb_fields?
Yeah, if you fix it then you should be all set as the change below would make it so that you could never match with the FM10K_VLAN_CLEAR bit set.
Quoted 15 lines
I just realized that is a bug in the
fm10k_process_skb_fieldsfunction.
You probably need to update it so that code does something more
likeif (rx_desc->w.vlan) { u16 vid = le16_to_cpu(rx_desc->w.vlan); if ((vid & VLAN_VID_MASK) != rx_ring->vid) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid); else if (vid & VLAN_PRIO_MASK) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid & VLAN_PRIO_MASK); }
Keller
PATCH 6/10 fm10k: send traffic on default vid to vlan device if we have one
Quoted 81 lines · Alexander Duyck
Quoted 80 lines · Jacob E
Quoted 79 lines · Alexander Duyck
Quoted 72 lines · Jacob Keller
This patch ensures that vlan traffic on the default vid will go to
the
corresponding vlan device if it exists. To do this, mask therx_ringvid
if we have an active vlan on that vid.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++ drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 4 ++++ 2 files changed, 16 insertions(+)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +0
-0@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct
net_device
*netdev, u16 vid, bool set)struct fm10k_intfc *interface = netdev_priv(netdev); struct fm10k_hw *hw = &interface->hw; s32 err;drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0+int i;/* updates do not apply to VLAN 0 */ if (!vid)@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct
net_device
*netdev, u16 vid, bool set)
if (!set)
clear_bit(vid, interface->active_vlans);drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1
-0+/* disable the default vid on ring if we have an active
vlan */drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +10
-1+for (i =0 ; i < interface->num_rx_queues; i++) { + struct fm10k_ring *rx_ring = interface ->rx_ring[i]; +u16 rx_vid = rx_ring->vid & (VLAN_N_VID -1 ); + +if (test_bit (rx_vid, interface->active_vlans)) + rx_ring->vid |=FM10K_VLAN_CLEAR ; +else + rx_ring->vid &= ~FM10K_VLAN_CLEAR ; + } +/* Do not remove default VID related entries from VLAN and
MAC tables */
if (!set && vid ==hw->mac.default_vid)
return 0;drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-1@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(structfm10k_intfc *interface,/* assign default VLAN to queue */ ring->vid = hw->mac.default_vid; +/* if we have an active VLAN, disable default vid */ + if (test_bit(hw->mac.default_vid, interface ->active_vlans)) + ring->vid |= FM10K_VLAN_CLEAR; + /* Map interrupt */ if (ring->q_vector) { rxint = ring->q_vector->v_idx +Quoted 2 lines
NON_Q_VECTORS(hw);This doesn't quite work since the
rx_ring->vidis doing a full 16b
comparison against a value that includes the QOS value.
Quoted 8 lines
Quoted 2 lines
To clarify, this should work as long as we fix the
process_skb_fields?Yeah, if you fix it then you should be all set as the change below would
make it so that you could never match with theFM10K_VLAN_CLEARbit set.
Yep, that's what i figured. I've sent new patch series with this fix in place.
Regards,
Jake
Quoted 16 lines
Quoted 15 lines
I just realized that is a bug in the
fm10k_process_skb_fieldsfunction.
You probably need to update it so that code does something more
likeif (rx_desc->w.vlan) { u16 vid = le16_to_cpu(rx_desc->w.vlan); if ((vid & VLAN_VID_MASK) != rx_ring->vid) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid); else if (vid & VLAN_PRIO_MASK) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid & VLAN_PRIO_MASK); }
PATCH 7/10 fm10k: TRIVIAL fix typo in fm10k_netdev.c
In reply to Jacob Keller--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +1 -1
@@ -1048,7 +1048,7 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface) vid, true, 0); } -/* update xcast mode before syncronizing addresses */ +/* update xcast mode before synchronizing addresses */ hw->mac.ops.update_xcast_mode (hw, glort, xcast_mode);/* synchronize all of the addresses */
Signature · 1 line
2.4.3
PATCH 8/10 fm10k: re-enable VF after a full reset on detection of a Malicious event
In reply to Jacob KellerModify behavior of Malicious Driver Detection events. Presently, the hardware disables the VF queues and re-assigns them to the PF. This causes the VF in question to continuously Tx hang, because it assumes that it can transmit over the queues in question. For transient events, this results in continuous logging of malicious events.
New behavior is to reset the LPORT and VF state, so that the VF will have to reset and re-enable itself. This does mean that malicious VFs will possibly be able to continue and attempt malicious events again. However, it is expected that system administrators will step in and manually remove or disable the VF in question.
--- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 30 ++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +28 -2
@@ -896,10 +896,12 @@ void fm10k_netpoll(struct net_device *netdev)#endif #define FM10K_ERR_MSG (type) case (type): error = #type; break -static void fm10k_print_fault (struct fm10k_intfc *interface,int type, +static void fm10k_handle_fault (struct fm10k_intfc *interface,int type,struct fm10k_fault *fault) {struct pci_dev *pdev = interface->pdev; +struct fm10k_hw *hw = &interface->hw; +struct fm10k_iov_data *iov_data = interface->iov_data;char *error;switch (type) {
@@ -953,6 +955,30 @@ static void fm10k_print_fault(struct fm10k_intfc *interface, int type,"%s Address: 0x%llx SpecInfo: 0x%x Func: %02x.%0x\n" , error, fault->address, fault->specinfo, PCI_SLOT(fault->func), PCI_FUNC(fault->func)); + +/* For VF faults, clear out the respective LPORT, reset the queue +* resources, and then reconnect to the mailbox. This allows the +* VF in question to resume behavior. For transient faults that are +* the result of non-malicious behavior this will log the fault and +* allow the VF to resume functionality. Obviously for malicious VFs +* they will be able to attempt malicious behavior again. In this +* case, the system administrator will need to step in and manually +* remove or disable the VF in question. +*/ +if (fault->func && iov_data) { +int vf = fault->func -1 ; +struct fm10k_vf_info *vf_info = &iov_data->vf_info[vf]; + + hw->iov.ops.reset_lport (hw, vf_info); + hw->iov.ops.reset_resources (hw, vf_info); + +/* reset_lport disables the VF, so re-enable it */ + hw->iov.ops.set_lport (hw, vf_info, vf, +FM10K_VF_FLAG_MULTI_CAPABLE ); + +/* reset_resources will disconnect from the mbx */ + vf_info->mbx.ops.connect (hw, &vf_info->mbx); + } }static void fm10k_report_fault (struct fm10k_intfc *interface,u32 eicr)
@@ -976,7 +1002,7 @@ static void fm10k_report_fault(struct fm10k_intfc *interface, u32 eicr)continue ; } -fm10k_print_fault (interface, type, &fault); +fm10k_handle_fault (interface, type, &fault); } }
Signature · 1 line
2.4.3
PATCH 9/10 fm10k: Only trigger data path reset if fabric is up
In reply to Jacob KellerFrom: Alexander Duyck <[email protected]>
This change makes it so that we only trigger the data path reset if the fabric is ready to handle traffic. The general idea is to avoid triggering the reset unless the switch API is ready for us. Otherwise we can just postpone the reset until we receive a switch ready notification.
--- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +6 -0
@@ -59,6 +59,11 @@ static s32 fm10k_reset_hw_pf(struct fm10k_hw *hw)if (reg & (FM10K_DMA_CTRL_TX_ACTIVE |FM10K_DMA_CTRL_RX_ACTIVE ))return FM10K_ERR_DMA_PENDING ; +/* verify the switch is ready for reset */ + reg =fm10k_read_reg (hw,FM10K_DMA_CTRL2 ); +if (!(reg &FM10K_DMA_CTRL2_SWITCH_READY )) + goto out; +/* Inititate data path reset */ reg |=FM10K_DMA_CTRL_DATAPATH_RESET ;fm10k_write_reg (hw,FM10K_DMA_CTRL , reg);
@@ -72,6 +77,7 @@ static s32 fm10k_reset_hw_pf(struct fm10k_hw *hw)if (!(reg &FM10K_IP_NOTINRESET )) err =FM10K_ERR_RESET_FAILED ; +out:return err; }
Signature · 1 line
2.4.3
PATCH 10/10 fm10k: fix iov_msg_mac_vlan_pf VID checks
In reply to Jacob KellerThe VF will send a message to request multicast addresses with the default vid. In the current code, if the PF has statically assigned a VLAN to a VF, then the VF will not get the multicast addresses. Fix up all of the various vlan messages to use identical checks (since each check was different). Also use set as a variable, so that it simplifies our check for whether vlan matches the pf_vid.
The new logic will allow set of a vlan if it is zero, automatically converting to the default vid. Otherwise it will allow setting the PF vid, or any VLAN if PF has not statically assigned a VLAN. This is consistent behavior, and allows VF to request either 0 or the default_vid without silently failing.
Note that we need the check for zero since VFs might not get the default VID message in time to actually request non-zero VLANs.
--- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +25 -12
@@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, struct fm10k_mbx_info *mbx) {struct fm10k_vf_info *vf_info = (struct fm10k_vf_info *)mbx; -int err =0 ;u8 mac[ETH_ALEN ];u32 *result; +int err =0 ; +bool set;u16 vlan;u32 vid;
@@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,if (err)return err; +/* verify upper 16 bits are zero */ +if (vid >>16 ) +return FM10K_ERR_PARAM ; + + set = !(vid &FM10K_VLAN_CLEAR ); + vid &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ -if (!vid || (vid ==FM10K_VLAN_CLEAR )) { +if (!vid) {if (vf_info->pf_vid) vid |= vf_info->pf_vid;else vid |= vf_info->sw_vid; - }else if (vid != vf_info->pf_vid) { + }else if (vf_info->pf_vid && vid != vf_info->pf_vid) {return FM10K_ERR_PARAM ; }/* update VSI info for VF in regards to VLAN table */ - err = hw->mac.ops.update_vlan (hw, vid, vf_info->vsi, - !(vid &FM10K_VLAN_CLEAR )); + err = hw->mac.ops.update_vlan (hw, vid, vf_info->vsi, set); }if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC ]) {
@@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, memcmp(mac, vf_info->mac, ETH_ALEN))return FM10K_ERR_PARAM ; + set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ -if (!vlan || (vlan ==FM10K_VLAN_CLEAR )) { +if (!vlan) {if (vf_info->pf_vid) vlan |= vf_info->pf_vid;else vlan |= vf_info->sw_vid; - }else if (vf_info->pf_vid) { + }else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {return FM10K_ERR_PARAM ; }/* notify switch of request for new unicast address */ - err = hw->mac.ops.update_uc_addr (hw, vf_info->glort, mac, vlan, - !(vlan &FM10K_VLAN_CLEAR ),0 ); + err = hw->mac.ops.update_uc_addr (hw, vf_info->glort, + mac, vlan, set,0 ); }if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST ]) {
@@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,if (!(vf_info->vf_flags &FM10K_VF_FLAG_MULTI_ENABLED ))return FM10K_ERR_PARAM ; + set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ if (!vlan || (vlan ==FM10K_VLAN_CLEAR )) {if (vf_info->pf_vid) vlan |= vf_info->pf_vid;else vlan |= vf_info->sw_vid; - }else if (vf_info->pf_vid) { + }else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {return FM10K_ERR_PARAM ; }/* notify switch of request for new multicast address */ - err = hw->mac.ops.update_mc_addr (hw, vf_info->glort, mac, vlan, - !(vlan &FM10K_VLAN_CLEAR )); + err = hw->mac.ops.update_mc_addr (hw, vf_info->glort, + mac, vlan, set); }return err;
Signature · 1 line
2.4.3
PATCH 10/10 fm10k: fix iov_msg_mac_vlan_pf VID checks
Quoted 95 lines · Jacob Keller
The VF will send a message to request multicast addresses with the default vid. In the current code, if the PF has statically assigned a VLAN to a VF, then the VF will not get the multicast addresses. Fix up all of the various vlan messages to use identical checks (since each check was different). Also use set as a variable, so that it simplifies our check for whether vlan matches the
pf_vid.The new logic will allow set of a vlan if it is zero, automatically converting to the default vid. Otherwise it will allow setting the PF vid, or any VLAN if PF has not statically assigned a VLAN. This is consistent behavior, and allows VF to request either 0 or the
default_vidwithout silently failing.Note that we need the check for zero since VFs might not get the default VID message in time to actually request non-zero VLANs.
Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_pf.c +19
-9@@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, struct fm10k_mbx_info *mbx) {struct fm10k_vf_info *vf_info = (struct fm10k_vf_info *)mbx; -int err =0 ;u8 mac[ETH_ALEN ];u32 *result; +int err =0 ; +bool set;u16 vlan;u32 vid;@@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,if (err)return err; +/* verify upper 16 bits are zero */ +if (vid >>16 ) +return FM10K_ERR_PARAM ; + + set = !(vid &FM10K_VLAN_CLEAR ); + vid &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ -if (!vid || (vid ==FM10K_VLAN_CLEAR )) { +if (!vid) {if (vf_info->pf_vid) vid |= vf_info->pf_vid;else vid |= vf_info->sw_vid; - }else if (vid != vf_info->pf_vid) { + }else if (vf_info->pf_vid && vid != vf_info->pf_vid) {return FM10K_ERR_PARAM ; }/* update VSI info for VF in regards to VLAN table */ - err = hw->mac.ops.update_vlan (hw, vid, vf_info->vsi, - !(vid &FM10K_VLAN_CLEAR )); + err = hw->mac.ops.update_vlan (hw, vid, vf_info->vsi, set); }if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC ]) {@@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, memcmp(mac, vf_info->mac, ETH_ALEN))return FM10K_ERR_PARAM ; + set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ -if (!vlan || (vlan ==FM10K_VLAN_CLEAR )) { +if (!vlan) {if (vf_info->pf_vid) vlan |= vf_info->pf_vid;else vlan |= vf_info->sw_vid; - }else if (vf_info->pf_vid) { + }else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {return FM10K_ERR_PARAM ; }/* notify switch of request for new unicast address */ - err = hw->mac.ops.update_uc_addr (hw, vf_info->glort, mac, vlan, - !(vlan &FM10K_VLAN_CLEAR ),0 ); + err = hw->mac.ops.update_uc_addr (hw, vf_info->glort, + mac, vlan, set,0 ); }if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST ]) {
drivers/net/ethernet/intel/fm10k/fm10k_pf.c s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, · line 1258
Quoted 8 lines
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +3
-0@@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,if (!(vf_info->vf_flags &FM10K_VF_FLAG_MULTI_ENABLED ))return FM10K_ERR_PARAM ; + set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */ if (!vlan || (vlan ==FM10K_VLAN_CLEAR )) {
You missed a spot here. This should be if(!vlan) since you already cleared the FM10K_VLAN_CLEAR flag.
Quoted 8 lines
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +1
-1if (vf_info->pf_vid) vlan |= vf_info->pf_vid;else vlan |= vf_info->sw_vid; - }else if (vf_info->pf_vid) { + }else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {return FM10K_ERR_PARAM ;
drivers/net/ethernet/intel/fm10k/fm10k_pf.c s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, · line 1272
Quoted 10 lines
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-2}/* notify switch of request for new multicast address */ - err = hw->mac.ops.update_mc_addr (hw, vf_info->glort, mac, vlan, - !(vlan &FM10K_VLAN_CLEAR )); + err = hw->mac.ops.update_mc_addr (hw, vf_info->glort, + mac, vlan, set); }return err;
Really since the code is so identical you might just want to create a function to do all of this. You could have it return a value representing the VLAN ID if >=0, or error if < 0.
Keller
PATCH 10/10 fm10k: fix iov_msg_mac_vlan_pf VID checks
Quoted 126 lines · Alexander Duyck
Quoted 125 lines · Jacob Keller
The VF will send a message to request multicast addresses with the default vid. In the current code, if the PF has statically assigned a
VLAN to a VF, then the VF will not get the multicast addresses. Fix up
all of the various vlan messages to use identical checks (since each
check was different). Also use set as a variable, so that it simplifies
our check for whether vlan matches thepf_vid.The new logic will allow set of a vlan if it is zero, automatically converting to the default vid. Otherwise it will allow setting the PF
vid, or any VLAN if PF has not statically assigned a VLAN. This is consistent behavior, and allows VF to request either 0 or thedefault_vidwithout silently failing.Note that we need the check for zero since VFs might not get the default
VID message in time to actually request non-zero VLANs.Signed-off-by Jacob Keller <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-1@@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, struct fm10k_mbx_info *mbx) {struct fm10k_vf_info *vf_info = (struct fm10k_vf_info *)mbx; -int err =0 ;u8 mac[ETH_ALEN ];u32 *result; +int err =0 ; +bool set;u16 vlan;u32 vid;
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +7
-0@@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, if (err)return err; +/* verify upper 16 bits are zero */ +if (vid >>16 ) +return FM10K_ERR_PARAM ; + + set = !(vid &FM10K_VLAN_CLEAR ); + vid &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-3-if (!vid || (vid ==FM10K_VLAN_CLEAR )) { +if (!vid) {if (vf_info->pf_vid) vid |= vf_info->pf_vid;else vid |= vf_info->sw_vid; - }else if (vid != vf_info->pf_vid) { + } else if (vf_info->pf_vid && vid != vf_info ->pf_vid) {return FM10K_ERR_PARAM ; }/* update VSI info for VF in regards to VLAN table */
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0
-3- err = hw->mac.ops.update_vlan(hw, vid, vf_info ->vsi, - !(vid &
FM10K_VLAN_CLEAR));drivers/net/ethernet/intel/fm10k/fm10k_pf.c +4
-1+ err = hw->mac.ops.update_vlan(hw, vid, vf_info ->vsi, set); } if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC]) {@@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, memcmp(mac, vf_info->mac, ETH_ALEN))return FM10K_ERR_PARAM ; + set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-3-if (!vlan || (vlan ==FM10K_VLAN_CLEAR )) { +if (!vlan) {if (vf_info->pf_vid) vlan |= vf_info->pf_vid;else vlan |= vf_info->sw_vid; - }else if (vf_info->pf_vid) { + } else if (vf_info->pf_vid && vlan != vf_info ->pf_vid) {return FM10K_ERR_PARAM ; }/* notify switch of request for new unicast address */
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0
-3- err = hw->mac.ops.update_uc_addr(hw, vf_info ->glort, mac, vlan, - !(vlan &
FM10K_VLAN_CLEAR), 0);drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-1+ err = hw->mac.ops.update_uc_addr(hw, vf_info ->glort, + mac, vlan, set,0);
}if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST]) {drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0
-0@@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results, if (!(vf_info->vf_flags & FM10K_VF_FLAG_MULTI_ENABLED))return FM10K_ERR_PARAM ;
Quoted 10 lines
Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +3
-0+ set = !(vlan &FM10K_VLAN_CLEAR ); + vlan &= ~FM10K_VLAN_CLEAR ; +/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
if (!vlan || (vlan ==FM10K_VLAN_CLEAR)) {You missed a spot here. This should be if(!vlan) since you already cleared the
FM10K_VLAN_CLEARflag.
Oops yep.
Quoted 21 lines
Quoted 21 lines
if (vf_info->pf_vid) vlan |= vf_info->pf_vid; else vlan |= vf_info->sw_vid;drivers/net/ethernet/intel/fm10k/fm10k_pf.c +1
-2- }else if (vf_info->pf_vid) { + } else if (vf_info->pf_vid && vlan != vf_info ->pf_vid) {return FM10K_ERR_PARAM ; }/* notify switch of request for new multicast address */
drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0
-3- err = hw->mac.ops.update_mc_addr(hw, vf_info ->glort, mac, vlan, - !(vlan &
FM10K_VLAN_CLEAR));drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2
-1+ err = hw->mac.ops.update_mc_addr (hw, vf_info ->glort, + mac, vlan, set); }
Quoted 8 lines
Quoted 2 lines
return err;
Really since the code is so identical you might just want to create a
function to do all of this. You could have it return a value representing the VLAN ID if >=0, or error if < 0.
I tried that first, but the main issue was different types. I wasn't sure how type-casting from a 32 to a 16 would do the right thing. For some flows we have a 16bit value, for others a 32bit value. (since it stores extra data). Actually, just casting it to 16 should work right? and then we can have the function just take a 16bit.
I'll look at that.
Regards,
Jake