* [net-next 6/7] igb: Enable reading of wake up packet
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Kim Tatt Chuah, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Kim Tatt Chuah <kim.tatt.chuah@intel.com>
Currently, in igb_resume(), igb driver ignores the Wake Up Status (WUS)
and Wake Up Packet Memory (WUPM) registers. This patch enables the igb
driver to read the WUPM if the controller was woken by a wake up packet
that is not more than 128 bytes long (maximum WUPM size), then pass it
up the kernel network stack.
Signed-off-by: Kim Tatt Chuah <kim.tatt.chuah@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_defines.h | 21 +++++++++++++++
drivers/net/ethernet/intel/igb/igb_main.c | 36 +++++++++++++++++++++++++-
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 8aee314332a8..d8517779439b 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -39,6 +39,27 @@
#define E1000_WUFC_MC 0x00000008 /* Directed Multicast Wakeup Enable */
#define E1000_WUFC_BC 0x00000010 /* Broadcast Wakeup Enable */
+/* Wake Up Status */
+#define E1000_WUS_EX 0x00000004 /* Directed Exact */
+#define E1000_WUS_ARPD 0x00000020 /* Directed ARP Request */
+#define E1000_WUS_IPV4 0x00000040 /* Directed IPv4 */
+#define E1000_WUS_IPV6 0x00000080 /* Directed IPv6 */
+#define E1000_WUS_NSD 0x00000400 /* Directed IPv6 Neighbor Solicitation */
+
+/* Packet types that are enabled for wake packet delivery */
+#define WAKE_PKT_WUS ( \
+ E1000_WUS_EX | \
+ E1000_WUS_ARPD | \
+ E1000_WUS_IPV4 | \
+ E1000_WUS_IPV6 | \
+ E1000_WUS_NSD)
+
+/* Wake Up Packet Length */
+#define E1000_WUPL_MASK 0x00000FFF
+
+/* Wake Up Packet Memory stores the first 128 bytes of the wake up packet */
+#define E1000_WUPM_BYTES 128
+
/* Extended Device Control */
#define E1000_CTRL_EXT_SDP2_DATA 0x00000040 /* Value of SW Defineable Pin 2 */
#define E1000_CTRL_EXT_SDP3_DATA 0x00000080 /* Value of SW Defineable Pin 3 */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 53e66c87abaf..1cf74aa4ebd9 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7985,6 +7985,36 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake,
return 0;
}
+static void igb_deliver_wake_packet(struct net_device *netdev)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ struct e1000_hw *hw = &adapter->hw;
+ struct sk_buff *skb;
+ u32 wupl;
+
+ wupl = rd32(E1000_WUPL) & E1000_WUPL_MASK;
+
+ /* WUPM stores only the first 128 bytes of the wake packet.
+ * Read the packet only if we have the whole thing.
+ */
+ if ((wupl == 0) || (wupl > E1000_WUPM_BYTES))
+ return;
+
+ skb = netdev_alloc_skb_ip_align(netdev, E1000_WUPM_BYTES);
+ if (!skb)
+ return;
+
+ skb_put(skb, wupl);
+
+ /* Ensure reads are 32-bit aligned */
+ wupl = roundup(wupl, 4);
+
+ memcpy_fromio(skb->data, hw->hw_addr + E1000_WUPM_REG(0), wupl);
+
+ skb->protocol = eth_type_trans(skb, netdev);
+ netif_rx(skb);
+}
+
#ifdef CONFIG_PM
#ifdef CONFIG_PM_SLEEP
static int igb_suspend(struct device *dev)
@@ -8014,7 +8044,7 @@ static int igb_resume(struct device *dev)
struct net_device *netdev = pci_get_drvdata(pdev);
struct igb_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
- u32 err;
+ u32 err, val;
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
@@ -8045,6 +8075,10 @@ static int igb_resume(struct device *dev)
*/
igb_get_hw_control(adapter);
+ val = rd32(E1000_WUS);
+ if (val & WAKE_PKT_WUS)
+ igb_deliver_wake_packet(netdev);
+
wr32(E1000_WUS, ~0);
rtnl_lock();
--
2.12.2
^ permalink raw reply related
* [net-next 7/7] igbvf: Use net_device_stats from struct net_device
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Tobias Klauser, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Tobias Klauser <tklauser@distanz.ch>
Instead of using a private copy of struct net_device_stats in
struct igbvf_adapter, use stats from struct net_device. Also remove the
now unnecessary .ndo_get_stats function.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igbvf/igbvf.h | 1 -
drivers/net/ethernet/intel/igbvf/netdev.c | 26 +++++---------------------
2 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/intel/igbvf/igbvf.h b/drivers/net/ethernet/intel/igbvf/igbvf.h
index 2a53ed84d9fc..bf69f01f8467 100644
--- a/drivers/net/ethernet/intel/igbvf/igbvf.h
+++ b/drivers/net/ethernet/intel/igbvf/igbvf.h
@@ -243,7 +243,6 @@ struct igbvf_adapter {
/* OS defined structs */
struct net_device *netdev;
struct pci_dev *pdev;
- struct net_device_stats net_stats;
spinlock_t stats_lock; /* prevent concurrent stats updates */
/* structs defined in e1000_hw.h */
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 6029854b6a70..1b9cbbe88f6f 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -400,8 +400,8 @@ static bool igbvf_clean_rx_irq(struct igbvf_adapter *adapter,
adapter->total_rx_packets += total_packets;
adapter->total_rx_bytes += total_bytes;
- adapter->net_stats.rx_bytes += total_bytes;
- adapter->net_stats.rx_packets += total_packets;
+ netdev->stats.rx_bytes += total_bytes;
+ netdev->stats.rx_packets += total_packets;
return cleaned;
}
@@ -864,8 +864,8 @@ static bool igbvf_clean_tx_irq(struct igbvf_ring *tx_ring)
}
}
- adapter->net_stats.tx_bytes += total_bytes;
- adapter->net_stats.tx_packets += total_packets;
+ netdev->stats.tx_bytes += total_bytes;
+ netdev->stats.tx_packets += total_packets;
return count < tx_ring->count;
}
@@ -1838,7 +1838,7 @@ void igbvf_update_stats(struct igbvf_adapter *adapter)
UPDATE_VF_COUNTER(VFGPRLBC, gprlbc);
/* Fill out the OS statistics structure */
- adapter->net_stats.multicast = adapter->stats.mprc;
+ adapter->netdev->stats.multicast = adapter->stats.mprc;
}
static void igbvf_print_link_info(struct igbvf_adapter *adapter)
@@ -2374,21 +2374,6 @@ static void igbvf_reset_task(struct work_struct *work)
}
/**
- * igbvf_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the timer callback.
- **/
-static struct net_device_stats *igbvf_get_stats(struct net_device *netdev)
-{
- struct igbvf_adapter *adapter = netdev_priv(netdev);
-
- /* only return the current stats */
- return &adapter->net_stats;
-}
-
-/**
* igbvf_change_mtu - Change the Maximum Transfer Unit
* @netdev: network interface device structure
* @new_mtu: new value for maximum frame size
@@ -2675,7 +2660,6 @@ static const struct net_device_ops igbvf_netdev_ops = {
.ndo_open = igbvf_open,
.ndo_stop = igbvf_close,
.ndo_start_xmit = igbvf_xmit_frame,
- .ndo_get_stats = igbvf_get_stats,
.ndo_set_rx_mode = igbvf_set_rx_mode,
.ndo_set_mac_address = igbvf_set_mac,
.ndo_change_mtu = igbvf_change_mtu,
--
2.12.2
^ permalink raw reply related
* [net-next 5/7] igb/igbvf: Add VF MAC filter request capabilities
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Yury Kylulin, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Yury Kylulin <yury.kylulin@intel.com>
Add functionality for the VF to request up to 3 additional MAC filters.
This is done using existing E1000_VF_SET_MAC_ADDR message, but with
additional message info - E1000_VF_MAC_FILTER_CLR to clear all unicast
MAC filters previously set for this VF and E1000_VF_MAC_FILTER_ADD to
add MAC filter.
Additional filters can be added only in case if administrator did not
set VF MAC explicitly and allowed to change default MAC to the VF.
Due to the limited number of RAR entries reserve at least 3 MAC filters
for the PF.
If SRIOV is supported by the NIC after this change RAR entries starting
from 1 to (RAR MAX ENTRIES - NUM SRIOV VFS) will be used for PF and VF
MAC filters.
Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_mbx.h | 4 +
drivers/net/ethernet/intel/igb/igb.h | 12 +++
drivers/net/ethernet/intel/igb/igb_main.c | 147 ++++++++++++++++++++++++++---
drivers/net/ethernet/intel/igbvf/igbvf.h | 2 +
drivers/net/ethernet/intel/igbvf/mbx.h | 4 +
drivers/net/ethernet/intel/igbvf/netdev.c | 44 ++++++++-
drivers/net/ethernet/intel/igbvf/vf.c | 41 ++++++++
drivers/net/ethernet/intel/igbvf/vf.h | 1 +
8 files changed, 240 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.h b/drivers/net/ethernet/intel/igb/e1000_mbx.h
index d20af6b2f581..3e7fed73df15 100644
--- a/drivers/net/ethernet/intel/igb/e1000_mbx.h
+++ b/drivers/net/ethernet/intel/igb/e1000_mbx.h
@@ -55,6 +55,10 @@
#define E1000_VF_RESET 0x01 /* VF requests reset */
#define E1000_VF_SET_MAC_ADDR 0x02 /* VF requests to set MAC addr */
+/* VF requests to clear all unicast MAC filters */
+#define E1000_VF_MAC_FILTER_CLR (0x01 << E1000_VT_MSGINFO_SHIFT)
+/* VF requests to add unicast MAC filter */
+#define E1000_VF_MAC_FILTER_ADD (0x02 << E1000_VT_MSGINFO_SHIFT)
#define E1000_VF_SET_MULTICAST 0x03 /* VF requests to set MC addr */
#define E1000_VF_SET_VLAN 0x04 /* VF requests to set VLAN */
#define E1000_VF_SET_LPE 0x05 /* VF requests to set VMOLR.LPE */
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 97f348aa6ba4..bf9bf9056d0c 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -111,6 +111,16 @@ struct vf_data_storage {
bool spoofchk_enabled;
};
+/* Number of unicast MAC filters reserved for the PF in the RAR registers */
+#define IGB_PF_MAC_FILTERS_RESERVED 3
+
+struct vf_mac_filter {
+ struct list_head l;
+ int vf;
+ bool free;
+ u8 vf_mac[ETH_ALEN];
+};
+
#define IGB_VF_FLAG_CTS 0x00000001 /* VF is clear to send data */
#define IGB_VF_FLAG_UNI_PROMISC 0x00000002 /* VF has unicast promisc */
#define IGB_VF_FLAG_MULTI_PROMISC 0x00000004 /* VF has multicast promisc */
@@ -586,6 +596,8 @@ struct igb_adapter {
bool etype_bitmap[MAX_ETYPE_FILTER];
struct igb_mac_addr *mac_table;
+ struct vf_mac_filter vf_macs;
+ struct vf_mac_filter *vf_mac_list;
};
/* flags controlling PTP/1588 function */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index bb5d5ac22f04..53e66c87abaf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1158,6 +1158,8 @@ static void igb_set_interrupt_capability(struct igb_adapter *adapter, bool msix)
pci_disable_sriov(adapter->pdev);
msleep(500);
+ kfree(adapter->vf_mac_list);
+ adapter->vf_mac_list = NULL;
kfree(adapter->vf_data);
adapter->vf_data = NULL;
wr32(E1000_IOVCTL, E1000_IOVCTL_REUSE_VFQ);
@@ -2809,6 +2811,8 @@ static int igb_disable_sriov(struct pci_dev *pdev)
msleep(500);
}
+ kfree(adapter->vf_mac_list);
+ adapter->vf_mac_list = NULL;
kfree(adapter->vf_data);
adapter->vf_data = NULL;
adapter->vfs_allocated_count = 0;
@@ -2829,8 +2833,9 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
struct net_device *netdev = pci_get_drvdata(pdev);
struct igb_adapter *adapter = netdev_priv(netdev);
int old_vfs = pci_num_vf(pdev);
+ struct vf_mac_filter *mac_list;
int err = 0;
- int i;
+ int num_vf_mac_filters, i;
if (!(adapter->flags & IGB_FLAG_HAS_MSIX) || num_vfs > 7) {
err = -EPERM;
@@ -2858,6 +2863,38 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
goto out;
}
+ /* Due to the limited number of RAR entries calculate potential
+ * number of MAC filters available for the VFs. Reserve entries
+ * for PF default MAC, PF MAC filters and at least one RAR entry
+ * for each VF for VF MAC.
+ */
+ num_vf_mac_filters = adapter->hw.mac.rar_entry_count -
+ (1 + IGB_PF_MAC_FILTERS_RESERVED +
+ adapter->vfs_allocated_count);
+
+ adapter->vf_mac_list = kcalloc(num_vf_mac_filters,
+ sizeof(struct vf_mac_filter),
+ GFP_KERNEL);
+
+ mac_list = adapter->vf_mac_list;
+ INIT_LIST_HEAD(&adapter->vf_macs.l);
+
+ if (adapter->vf_mac_list) {
+ /* Initialize list of VF MAC filters */
+ for (i = 0; i < num_vf_mac_filters; i++) {
+ mac_list->vf = -1;
+ mac_list->free = true;
+ list_add(&mac_list->l, &adapter->vf_macs.l);
+ mac_list++;
+ }
+ } else {
+ /* If we could not allocate memory for the VF MAC filters
+ * we can continue without this feature but warn user.
+ */
+ dev_err(&pdev->dev,
+ "Unable to allocate memory for VF MAC filter list\n");
+ }
+
/* only call pci_enable_sriov() if no VFs are allocated already */
if (!old_vfs) {
err = pci_enable_sriov(pdev, adapter->vfs_allocated_count);
@@ -2874,6 +2911,8 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
goto out;
err_out:
+ kfree(adapter->vf_mac_list);
+ adapter->vf_mac_list = NULL;
kfree(adapter->vf_data);
adapter->vf_data = NULL;
adapter->vfs_allocated_count = 0;
@@ -6501,18 +6540,106 @@ static int igb_uc_unsync(struct net_device *netdev, const unsigned char *addr)
return 0;
}
+int igb_set_vf_mac_filter(struct igb_adapter *adapter, const int vf,
+ const u32 info, const u8 *addr)
+{
+ struct pci_dev *pdev = adapter->pdev;
+ struct vf_data_storage *vf_data = &adapter->vf_data[vf];
+ struct list_head *pos;
+ struct vf_mac_filter *entry = NULL;
+ int ret = 0;
+
+ switch (info) {
+ case E1000_VF_MAC_FILTER_CLR:
+ /* remove all unicast MAC filters related to the current VF */
+ list_for_each(pos, &adapter->vf_macs.l) {
+ entry = list_entry(pos, struct vf_mac_filter, l);
+ if (entry->vf == vf) {
+ entry->vf = -1;
+ entry->free = true;
+ igb_del_mac_filter(adapter, entry->vf_mac, vf);
+ }
+ }
+ break;
+ case E1000_VF_MAC_FILTER_ADD:
+ if (vf_data->flags & IGB_VF_FLAG_PF_SET_MAC) {
+ dev_warn(&pdev->dev,
+ "VF %d requested MAC filter but is administratively denied\n",
+ vf);
+ return -EINVAL;
+ }
+
+ if (!is_valid_ether_addr(addr)) {
+ dev_warn(&pdev->dev,
+ "VF %d attempted to set invalid MAC filter\n",
+ vf);
+ return -EINVAL;
+ }
+
+ /* try to find empty slot in the list */
+ list_for_each(pos, &adapter->vf_macs.l) {
+ entry = list_entry(pos, struct vf_mac_filter, l);
+ if (entry->free)
+ break;
+ }
+
+ if (entry && entry->free) {
+ entry->free = false;
+ entry->vf = vf;
+ ether_addr_copy(entry->vf_mac, addr);
+
+ ret = igb_add_mac_filter(adapter, addr, vf);
+ ret = min_t(int, ret, 0);
+ } else {
+ ret = -ENOSPC;
+ }
+
+ if (ret == -ENOSPC)
+ dev_warn(&pdev->dev,
+ "VF %d has requested MAC filter but there is no space for it\n",
+ vf);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
static int igb_set_vf_mac_addr(struct igb_adapter *adapter, u32 *msg, int vf)
{
+ struct pci_dev *pdev = adapter->pdev;
+ struct vf_data_storage *vf_data = &adapter->vf_data[vf];
+ u32 info = msg[0] & E1000_VT_MSGINFO_MASK;
+
/* The VF MAC Address is stored in a packed array of bytes
* starting at the second 32 bit word of the msg array
*/
- unsigned char *addr = (char *)&msg[1];
- int err = -1;
+ unsigned char *addr = (unsigned char *)&msg[1];
+ int ret = 0;
- if (is_valid_ether_addr(addr))
- err = igb_set_vf_mac(adapter, vf, addr);
+ if (!info) {
+ if (vf_data->flags & IGB_VF_FLAG_PF_SET_MAC) {
+ dev_warn(&pdev->dev,
+ "VF %d attempted to override administratively set MAC address\nReload the VF driver to resume operations\n",
+ vf);
+ return -EINVAL;
+ }
- return err;
+ if (!is_valid_ether_addr(addr)) {
+ dev_warn(&pdev->dev,
+ "VF %d attempted to set invalid MAC\n",
+ vf);
+ return -EINVAL;
+ }
+
+ ret = igb_set_vf_mac(adapter, vf, addr);
+ } else {
+ ret = igb_set_vf_mac_filter(adapter, vf, info, addr);
+ }
+
+ return ret;
}
static void igb_rcv_ack_from_vf(struct igb_adapter *adapter, u32 vf)
@@ -6569,13 +6696,7 @@ static void igb_rcv_msg_from_vf(struct igb_adapter *adapter, u32 vf)
switch ((msgbuf[0] & 0xFFFF)) {
case E1000_VF_SET_MAC_ADDR:
- retval = -EINVAL;
- if (!(vf_data->flags & IGB_VF_FLAG_PF_SET_MAC))
- retval = igb_set_vf_mac_addr(adapter, msgbuf, vf);
- else
- dev_warn(&pdev->dev,
- "VF %d attempted to override administratively set MAC address\nReload the VF driver to resume operations\n",
- vf);
+ retval = igb_set_vf_mac_addr(adapter, msgbuf, vf);
break;
case E1000_VF_SET_PROMISC:
retval = igb_set_vf_promisc(adapter, msgbuf, vf);
diff --git a/drivers/net/ethernet/intel/igbvf/igbvf.h b/drivers/net/ethernet/intel/igbvf/igbvf.h
index 6f4290d6dc9f..2a53ed84d9fc 100644
--- a/drivers/net/ethernet/intel/igbvf/igbvf.h
+++ b/drivers/net/ethernet/intel/igbvf/igbvf.h
@@ -101,6 +101,8 @@ enum latency_range {
#define IGBVF_MNG_VLAN_NONE (-1)
+#define IGBVF_MAX_MAC_FILTERS 3
+
/* Number of packet split data buffers (not including the header buffer) */
#define PS_PAGE_BUFFERS (MAX_PS_BUFFERS - 1)
diff --git a/drivers/net/ethernet/intel/igbvf/mbx.h b/drivers/net/ethernet/intel/igbvf/mbx.h
index f800bf8eedae..30d58c4a444e 100644
--- a/drivers/net/ethernet/intel/igbvf/mbx.h
+++ b/drivers/net/ethernet/intel/igbvf/mbx.h
@@ -62,6 +62,10 @@
#define E1000_VF_RESET 0x01 /* VF requests reset */
#define E1000_VF_SET_MAC_ADDR 0x02 /* VF requests PF to set MAC addr */
+/* VF requests PF to clear all unicast MAC filters */
+#define E1000_VF_MAC_FILTER_CLR (0x01 << E1000_VT_MSGINFO_SHIFT)
+/* VF requests PF to add unicast MAC filter */
+#define E1000_VF_MAC_FILTER_ADD (0x02 << E1000_VT_MSGINFO_SHIFT)
#define E1000_VF_SET_MULTICAST 0x03 /* VF requests PF to set MC addr */
#define E1000_VF_SET_VLAN 0x04 /* VF requests PF to set VLAN */
#define E1000_VF_SET_LPE 0x05 /* VF requests PF to set VMOLR.LPE */
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 839ba110f7fb..6029854b6a70 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -1433,12 +1433,52 @@ static void igbvf_set_multi(struct net_device *netdev)
}
/**
+ * igbvf_set_uni - Configure unicast MAC filters
+ * @netdev: network interface device structure
+ *
+ * This routine is responsible for configuring the hardware for proper
+ * unicast filters.
+ **/
+static int igbvf_set_uni(struct net_device *netdev)
+{
+ struct igbvf_adapter *adapter = netdev_priv(netdev);
+ struct e1000_hw *hw = &adapter->hw;
+
+ if (netdev_uc_count(netdev) > IGBVF_MAX_MAC_FILTERS) {
+ pr_err("Too many unicast filters - No Space\n");
+ return -ENOSPC;
+ }
+
+ /* Clear all unicast MAC filters */
+ hw->mac.ops.set_uc_addr(hw, E1000_VF_MAC_FILTER_CLR, NULL);
+
+ if (!netdev_uc_empty(netdev)) {
+ struct netdev_hw_addr *ha;
+
+ /* Add MAC filters one by one */
+ netdev_for_each_uc_addr(ha, netdev) {
+ hw->mac.ops.set_uc_addr(hw, E1000_VF_MAC_FILTER_ADD,
+ ha->addr);
+ udelay(200);
+ }
+ }
+
+ return 0;
+}
+
+static void igbvf_set_rx_mode(struct net_device *netdev)
+{
+ igbvf_set_multi(netdev);
+ igbvf_set_uni(netdev);
+}
+
+/**
* igbvf_configure - configure the hardware for Rx and Tx
* @adapter: private board structure
**/
static void igbvf_configure(struct igbvf_adapter *adapter)
{
- igbvf_set_multi(adapter->netdev);
+ igbvf_set_rx_mode(adapter->netdev);
igbvf_restore_vlan(adapter);
@@ -2636,7 +2676,7 @@ static const struct net_device_ops igbvf_netdev_ops = {
.ndo_stop = igbvf_close,
.ndo_start_xmit = igbvf_xmit_frame,
.ndo_get_stats = igbvf_get_stats,
- .ndo_set_rx_mode = igbvf_set_multi,
+ .ndo_set_rx_mode = igbvf_set_rx_mode,
.ndo_set_mac_address = igbvf_set_mac,
.ndo_change_mtu = igbvf_change_mtu,
.ndo_do_ioctl = igbvf_ioctl,
diff --git a/drivers/net/ethernet/intel/igbvf/vf.c b/drivers/net/ethernet/intel/igbvf/vf.c
index 335ba6642145..528be116184e 100644
--- a/drivers/net/ethernet/intel/igbvf/vf.c
+++ b/drivers/net/ethernet/intel/igbvf/vf.c
@@ -36,6 +36,7 @@ static void e1000_update_mc_addr_list_vf(struct e1000_hw *hw, u8 *,
u32, u32, u32);
static void e1000_rar_set_vf(struct e1000_hw *, u8 *, u32);
static s32 e1000_read_mac_addr_vf(struct e1000_hw *);
+static s32 e1000_set_uc_addr_vf(struct e1000_hw *hw, u32 subcmd, u8 *addr);
static s32 e1000_set_vfta_vf(struct e1000_hw *, u16, bool);
/**
@@ -66,6 +67,8 @@ static s32 e1000_init_mac_params_vf(struct e1000_hw *hw)
mac->ops.rar_set = e1000_rar_set_vf;
/* read mac address */
mac->ops.read_mac_addr = e1000_read_mac_addr_vf;
+ /* set mac filter */
+ mac->ops.set_uc_addr = e1000_set_uc_addr_vf;
/* set vlan filter table array */
mac->ops.set_vfta = e1000_set_vfta_vf;
@@ -338,6 +341,44 @@ static s32 e1000_read_mac_addr_vf(struct e1000_hw *hw)
}
/**
+ * e1000_set_uc_addr_vf - Set or clear unicast filters
+ * @hw: pointer to the HW structure
+ * @sub_cmd: add or clear filters
+ * @addr: pointer to the filter MAC address
+ **/
+static s32 e1000_set_uc_addr_vf(struct e1000_hw *hw, u32 sub_cmd, u8 *addr)
+{
+ struct e1000_mbx_info *mbx = &hw->mbx;
+ u32 msgbuf[3], msgbuf_chk;
+ u8 *msg_addr = (u8 *)(&msgbuf[1]);
+ s32 ret_val;
+
+ memset(msgbuf, 0, sizeof(msgbuf));
+ msgbuf[0] |= sub_cmd;
+ msgbuf[0] |= E1000_VF_SET_MAC_ADDR;
+ msgbuf_chk = msgbuf[0];
+
+ if (addr)
+ memcpy(msg_addr, addr, ETH_ALEN);
+
+ ret_val = mbx->ops.write_posted(hw, msgbuf, 3);
+
+ if (!ret_val)
+ ret_val = mbx->ops.read_posted(hw, msgbuf, 3);
+
+ msgbuf[0] &= ~E1000_VT_MSGTYPE_CTS;
+
+ if (!ret_val) {
+ msgbuf[0] &= ~E1000_VT_MSGTYPE_CTS;
+
+ if (msgbuf[0] == (msgbuf_chk | E1000_VT_MSGTYPE_NACK))
+ return -ENOSPC;
+ }
+
+ return ret_val;
+}
+
+/**
* e1000_check_for_link_vf - Check for link for a virtual interface
* @hw: pointer to the HW structure
*
diff --git a/drivers/net/ethernet/intel/igbvf/vf.h b/drivers/net/ethernet/intel/igbvf/vf.h
index f00a41d9a1ca..4cf78b0dec50 100644
--- a/drivers/net/ethernet/intel/igbvf/vf.h
+++ b/drivers/net/ethernet/intel/igbvf/vf.h
@@ -179,6 +179,7 @@ struct e1000_mac_operations {
s32 (*get_bus_info)(struct e1000_hw *);
s32 (*get_link_up_info)(struct e1000_hw *, u16 *, u16 *);
void (*update_mc_addr_list)(struct e1000_hw *, u8 *, u32, u32, u32);
+ s32 (*set_uc_addr)(struct e1000_hw *, u32, u8 *);
s32 (*reset_hw)(struct e1000_hw *);
s32 (*init_hw)(struct e1000_hw *);
s32 (*setup_link)(struct e1000_hw *);
--
2.12.2
^ permalink raw reply related
* [net-next 4/7] igb: improve MAC filter handling
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Yury Kylulin, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Yury Kylulin <yury.kylulin@intel.com>
Using the work which was done for ixgbe driver by Jacob Keller
commit 5d7daa35b9eb ("ixgbe: improve mac filter handling") and Alexander
Duyck commit 0f079d22834a ("ixgbe: Use __dev_uc_sync and __dev_uc_unsync
for unicast addresses") and out-of-tree igb driver add functionality to
manage (add and delete) MAC filters.
Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 11 ++
drivers/net/ethernet/intel/igb/igb_main.c | 246 ++++++++++++++++++++++--------
2 files changed, 192 insertions(+), 65 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index dc6e2980718f..97f348aa6ba4 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -449,6 +449,15 @@ struct igb_nfc_filter {
u16 action;
};
+struct igb_mac_addr {
+ u8 addr[ETH_ALEN];
+ u8 queue;
+ u8 state; /* bitmask */
+};
+
+#define IGB_MAC_STATE_DEFAULT 0x1
+#define IGB_MAC_STATE_IN_USE 0x2
+
/* board specific private data structure */
struct igb_adapter {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
@@ -575,6 +584,8 @@ struct igb_adapter {
/* lock for RX network flow classification filter */
spinlock_t nfc_lock;
bool etype_bitmap[MAX_ETYPE_FILTER];
+
+ struct igb_mac_addr *mac_table;
};
/* flags controlling PTP/1588 function */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 26a821fcd220..bb5d5ac22f04 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -161,11 +161,16 @@ static void igb_vlan_mode(struct net_device *netdev,
static int igb_vlan_rx_add_vid(struct net_device *, __be16, u16);
static int igb_vlan_rx_kill_vid(struct net_device *, __be16, u16);
static void igb_restore_vlan(struct igb_adapter *);
-static void igb_rar_set_qsel(struct igb_adapter *, u8 *, u32 , u8);
+static void igb_rar_set_index(struct igb_adapter *, u32);
static void igb_ping_all_vfs(struct igb_adapter *);
static void igb_msg_task(struct igb_adapter *);
static void igb_vmm_control(struct igb_adapter *);
static int igb_set_vf_mac(struct igb_adapter *, int, unsigned char *);
+static void igb_flush_mac_table(struct igb_adapter *);
+static int igb_available_rars(struct igb_adapter *, u8);
+static void igb_set_default_mac_filter(struct igb_adapter *);
+static int igb_uc_sync(struct net_device *, const unsigned char *);
+static int igb_uc_unsync(struct net_device *, const unsigned char *);
static void igb_restore_vf_multicasts(struct igb_adapter *adapter);
static int igb_ndo_set_vf_mac(struct net_device *netdev, int vf, u8 *mac);
static int igb_ndo_set_vf_vlan(struct net_device *netdev,
@@ -1987,6 +1992,13 @@ void igb_reset(struct igb_adapter *adapter)
if (hw->mac.ops.init_hw(hw))
dev_err(&pdev->dev, "Hardware Error\n");
+ /* RAR registers were cleared during init_hw, clear mac table */
+ igb_flush_mac_table(adapter);
+ __dev_uc_unsync(adapter->netdev, NULL);
+
+ /* Recover default RAR entry */
+ igb_set_default_mac_filter(adapter);
+
/* Flow control settings reset on hardware reset, so guarantee flow
* control is off when forcing speed.
*/
@@ -2095,11 +2107,9 @@ static int igb_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
/* guarantee we can provide a unique filter for the unicast address */
if (is_unicast_ether_addr(addr) || is_link_local_ether_addr(addr)) {
struct igb_adapter *adapter = netdev_priv(dev);
- struct e1000_hw *hw = &adapter->hw;
int vfn = adapter->vfs_allocated_count;
- int rar_entries = hw->mac.rar_entry_count - (vfn + 1);
- if (netdev_uc_count(dev) >= rar_entries)
+ if (netdev_uc_count(dev) >= igb_available_rars(adapter, vfn))
return -ENOMEM;
}
@@ -2517,6 +2527,8 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_eeprom;
}
+ igb_set_default_mac_filter(adapter);
+
/* get firmware version for ethtool -i */
igb_set_fw_version(adapter);
@@ -2761,6 +2773,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (hw->flash_address)
iounmap(hw->flash_address);
err_sw_init:
+ kfree(adapter->mac_table);
kfree(adapter->shadow_vfta);
igb_clear_interrupt_scheme(adapter);
#ifdef CONFIG_PCI_IOV
@@ -2937,6 +2950,7 @@ static void igb_remove(struct pci_dev *pdev)
iounmap(hw->flash_address);
pci_release_mem_regions(pdev);
+ kfree(adapter->mac_table);
kfree(adapter->shadow_vfta);
free_netdev(netdev);
@@ -3099,6 +3113,11 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* Assume MSI-X interrupts, will be checked during IRQ allocation */
adapter->flags |= IGB_FLAG_HAS_MSIX;
+ adapter->mac_table = kzalloc(sizeof(struct igb_mac_addr) *
+ hw->mac.rar_entry_count, GFP_ATOMIC);
+ if (!adapter->mac_table)
+ return -ENOMEM;
+
igb_probe_vfs(adapter);
igb_init_queue_configuration(adapter);
@@ -3810,8 +3829,7 @@ static void igb_configure_rx(struct igb_adapter *adapter)
int i;
/* set the correct pool for the PF default MAC address in entry 0 */
- igb_rar_set_qsel(adapter, adapter->hw.mac.addr, 0,
- adapter->vfs_allocated_count);
+ igb_set_default_mac_filter(adapter);
/* Setup the HW Rx Head and Tail Descriptor Pointers and
* the Base and Length of the Rx Descriptor Ring
@@ -4051,8 +4069,7 @@ static int igb_set_mac(struct net_device *netdev, void *p)
memcpy(hw->mac.addr, addr->sa_data, netdev->addr_len);
/* set the correct pool for the new PF MAC address in entry 0 */
- igb_rar_set_qsel(adapter, hw->mac.addr, 0,
- adapter->vfs_allocated_count);
+ igb_set_default_mac_filter(adapter);
return 0;
}
@@ -4096,49 +4113,6 @@ static int igb_write_mc_addr_list(struct net_device *netdev)
return netdev_mc_count(netdev);
}
-/**
- * igb_write_uc_addr_list - write unicast addresses to RAR table
- * @netdev: network interface device structure
- *
- * Writes unicast address list to the RAR table.
- * Returns: -ENOMEM on failure/insufficient address space
- * 0 on no addresses written
- * X on writing X addresses to the RAR table
- **/
-static int igb_write_uc_addr_list(struct net_device *netdev)
-{
- struct igb_adapter *adapter = netdev_priv(netdev);
- struct e1000_hw *hw = &adapter->hw;
- unsigned int vfn = adapter->vfs_allocated_count;
- unsigned int rar_entries = hw->mac.rar_entry_count - (vfn + 1);
- int count = 0;
-
- /* return ENOMEM indicating insufficient memory for addresses */
- if (netdev_uc_count(netdev) > rar_entries)
- return -ENOMEM;
-
- if (!netdev_uc_empty(netdev) && rar_entries) {
- struct netdev_hw_addr *ha;
-
- netdev_for_each_uc_addr(ha, netdev) {
- if (!rar_entries)
- break;
- igb_rar_set_qsel(adapter, ha->addr,
- rar_entries--,
- vfn);
- count++;
- }
- }
- /* write the addresses in reverse order to avoid write combining */
- for (; rar_entries > 0 ; rar_entries--) {
- wr32(E1000_RAH(rar_entries), 0);
- wr32(E1000_RAL(rar_entries), 0);
- }
- wrfl();
-
- return count;
-}
-
static int igb_vlan_promisc_enable(struct igb_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
@@ -4311,8 +4285,7 @@ static void igb_set_rx_mode(struct net_device *netdev)
* sufficient space to store all the addresses then enable
* unicast promiscuous mode
*/
- count = igb_write_uc_addr_list(netdev);
- if (count < 0) {
+ if (__dev_uc_sync(netdev, igb_uc_sync, igb_uc_unsync)) {
rctl |= E1000_RCTL_UPE;
vmolr |= E1000_VMOLR_ROPE;
}
@@ -6369,7 +6342,6 @@ static void igb_vf_reset_msg(struct igb_adapter *adapter, u32 vf)
{
struct e1000_hw *hw = &adapter->hw;
unsigned char *vf_mac = adapter->vf_data[vf].vf_mac_addresses;
- int rar_entry = hw->mac.rar_entry_count - (vf + 1);
u32 reg, msgbuf[3];
u8 *addr = (u8 *)(&msgbuf[1]);
@@ -6377,7 +6349,7 @@ static void igb_vf_reset_msg(struct igb_adapter *adapter, u32 vf)
igb_vf_reset(adapter, vf);
/* set vf mac address */
- igb_rar_set_qsel(adapter, vf_mac, rar_entry, vf);
+ igb_set_vf_mac(adapter, vf, vf_mac);
/* enable transmit and receive for vf */
reg = rd32(E1000_VFTE);
@@ -6397,6 +6369,138 @@ static void igb_vf_reset_msg(struct igb_adapter *adapter, u32 vf)
igb_write_mbx(hw, msgbuf, 3, vf);
}
+static void igb_flush_mac_table(struct igb_adapter *adapter)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ int i;
+
+ for (i = 0; i < hw->mac.rar_entry_count; i++) {
+ adapter->mac_table[i].state &= ~IGB_MAC_STATE_IN_USE;
+ memset(adapter->mac_table[i].addr, 0, ETH_ALEN);
+ adapter->mac_table[i].queue = 0;
+ igb_rar_set_index(adapter, i);
+ }
+}
+
+static int igb_available_rars(struct igb_adapter *adapter, u8 queue)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ /* do not count rar entries reserved for VFs MAC addresses */
+ int rar_entries = hw->mac.rar_entry_count -
+ adapter->vfs_allocated_count;
+ int i, count = 0;
+
+ for (i = 0; i < rar_entries; i++) {
+ /* do not count default entries */
+ if (adapter->mac_table[i].state & IGB_MAC_STATE_DEFAULT)
+ continue;
+
+ /* do not count "in use" entries for different queues */
+ if ((adapter->mac_table[i].state & IGB_MAC_STATE_IN_USE) &&
+ (adapter->mac_table[i].queue != queue))
+ continue;
+
+ count++;
+ }
+
+ return count;
+}
+
+/* Set default MAC address for the PF in the first RAR entry */
+static void igb_set_default_mac_filter(struct igb_adapter *adapter)
+{
+ struct igb_mac_addr *mac_table = &adapter->mac_table[0];
+
+ ether_addr_copy(mac_table->addr, adapter->hw.mac.addr);
+ mac_table->queue = adapter->vfs_allocated_count;
+ mac_table->state = IGB_MAC_STATE_DEFAULT | IGB_MAC_STATE_IN_USE;
+
+ igb_rar_set_index(adapter, 0);
+}
+
+int igb_add_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+ const u8 queue)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ int rar_entries = hw->mac.rar_entry_count -
+ adapter->vfs_allocated_count;
+ int i;
+
+ if (is_zero_ether_addr(addr))
+ return -EINVAL;
+
+ /* Search for the first empty entry in the MAC table.
+ * Do not touch entries at the end of the table reserved for the VF MAC
+ * addresses.
+ */
+ for (i = 0; i < rar_entries; i++) {
+ if (adapter->mac_table[i].state & IGB_MAC_STATE_IN_USE)
+ continue;
+
+ ether_addr_copy(adapter->mac_table[i].addr, addr);
+ adapter->mac_table[i].queue = queue;
+ adapter->mac_table[i].state |= IGB_MAC_STATE_IN_USE;
+
+ igb_rar_set_index(adapter, i);
+ return i;
+ }
+
+ return -ENOSPC;
+}
+
+int igb_del_mac_filter(struct igb_adapter *adapter, const u8 *addr,
+ const u8 queue)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ int rar_entries = hw->mac.rar_entry_count -
+ adapter->vfs_allocated_count;
+ int i;
+
+ if (is_zero_ether_addr(addr))
+ return -EINVAL;
+
+ /* Search for matching entry in the MAC table based on given address
+ * and queue. Do not touch entries at the end of the table reserved
+ * for the VF MAC addresses.
+ */
+ for (i = 0; i < rar_entries; i++) {
+ if (!(adapter->mac_table[i].state & IGB_MAC_STATE_IN_USE))
+ continue;
+ if (adapter->mac_table[i].queue != queue)
+ continue;
+ if (!ether_addr_equal(adapter->mac_table[i].addr, addr))
+ continue;
+
+ adapter->mac_table[i].state &= ~IGB_MAC_STATE_IN_USE;
+ memset(adapter->mac_table[i].addr, 0, ETH_ALEN);
+ adapter->mac_table[i].queue = 0;
+
+ igb_rar_set_index(adapter, i);
+ return 0;
+ }
+
+ return -ENOENT;
+}
+
+static int igb_uc_sync(struct net_device *netdev, const unsigned char *addr)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ int ret;
+
+ ret = igb_add_mac_filter(adapter, addr, adapter->vfs_allocated_count);
+
+ return min_t(int, ret, 0);
+}
+
+static int igb_uc_unsync(struct net_device *netdev, const unsigned char *addr)
+{
+ struct igb_adapter *adapter = netdev_priv(netdev);
+
+ igb_del_mac_filter(adapter, addr, adapter->vfs_allocated_count);
+
+ return 0;
+}
+
static int igb_set_vf_mac_addr(struct igb_adapter *adapter, u32 *msg, int vf)
{
/* The VF MAC Address is stored in a packed array of bytes
@@ -8078,11 +8182,16 @@ static void igb_io_resume(struct pci_dev *pdev)
igb_get_hw_control(adapter);
}
-static void igb_rar_set_qsel(struct igb_adapter *adapter, u8 *addr, u32 index,
- u8 qsel)
+/**
+ * igb_rar_set_index - Sync RAL[index] and RAH[index] registers with MAC table
+ * @adapter: Pointer to adapter structure
+ * @index: Index of the RAR entry which need to be synced with MAC table
+ **/
+static void igb_rar_set_index(struct igb_adapter *adapter, u32 index)
{
struct e1000_hw *hw = &adapter->hw;
u32 rar_low, rar_high;
+ u8 *addr = adapter->mac_table[index].addr;
/* HW expects these to be in network order when they are plugged
* into the registers which are little endian. In order to guarantee
@@ -8093,12 +8202,16 @@ static void igb_rar_set_qsel(struct igb_adapter *adapter, u8 *addr, u32 index,
rar_high = le16_to_cpup((__le16 *)(addr + 4));
/* Indicate to hardware the Address is Valid. */
- rar_high |= E1000_RAH_AV;
+ if (adapter->mac_table[index].state & IGB_MAC_STATE_IN_USE) {
+ rar_high |= E1000_RAH_AV;
- if (hw->mac.type == e1000_82575)
- rar_high |= E1000_RAH_POOL_1 * qsel;
- else
- rar_high |= E1000_RAH_POOL_1 << qsel;
+ if (hw->mac.type == e1000_82575)
+ rar_high |= E1000_RAH_POOL_1 *
+ adapter->mac_table[index].queue;
+ else
+ rar_high |= E1000_RAH_POOL_1 <<
+ adapter->mac_table[index].queue;
+ }
wr32(E1000_RAL(index), rar_low);
wrfl();
@@ -8114,10 +8227,13 @@ static int igb_set_vf_mac(struct igb_adapter *adapter,
* towards the first, as a result a collision should not be possible
*/
int rar_entry = hw->mac.rar_entry_count - (vf + 1);
+ unsigned char *vf_mac_addr = adapter->vf_data[vf].vf_mac_addresses;
- memcpy(adapter->vf_data[vf].vf_mac_addresses, mac_addr, ETH_ALEN);
-
- igb_rar_set_qsel(adapter, mac_addr, rar_entry, vf);
+ ether_addr_copy(vf_mac_addr, mac_addr);
+ ether_addr_copy(adapter->mac_table[rar_entry].addr, mac_addr);
+ adapter->mac_table[rar_entry].queue = vf;
+ adapter->mac_table[rar_entry].state |= IGB_MAC_STATE_IN_USE;
+ igb_rar_set_index(adapter, rar_entry);
return 0;
}
--
2.12.2
^ permalink raw reply related
* [net-next 1/7] e1000: Omit private ndo_get_stats function
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Tobias Klauser, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Tobias Klauser <tklauser@distanz.ch>
e1000_get_stats() just returns dev->stats so we can leave it
out altogether and let dev_get_stats() do the job.
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_main.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 93fc6c67306b..bd8b05fe8258 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -131,7 +131,6 @@ static void e1000_watchdog(struct work_struct *work);
static void e1000_82547_tx_fifo_stall_task(struct work_struct *work);
static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
struct net_device *netdev);
-static struct net_device_stats *e1000_get_stats(struct net_device *netdev);
static int e1000_change_mtu(struct net_device *netdev, int new_mtu);
static int e1000_set_mac(struct net_device *netdev, void *p);
static irqreturn_t e1000_intr(int irq, void *data);
@@ -846,7 +845,6 @@ static const struct net_device_ops e1000_netdev_ops = {
.ndo_open = e1000_open,
.ndo_stop = e1000_close,
.ndo_start_xmit = e1000_xmit_frame,
- .ndo_get_stats = e1000_get_stats,
.ndo_set_rx_mode = e1000_set_rx_mode,
.ndo_set_mac_address = e1000_set_mac,
.ndo_tx_timeout = e1000_tx_timeout,
@@ -3530,19 +3528,6 @@ static void e1000_reset_task(struct work_struct *work)
}
/**
- * e1000_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the watchdog.
- **/
-static struct net_device_stats *e1000_get_stats(struct net_device *netdev)
-{
- /* only return the current stats */
- return &netdev->stats;
-}
-
-/**
* e1000_change_mtu - Change the Maximum Transfer Unit
* @netdev: network interface device structure
* @new_mtu: new value for maximum frame size
--
2.12.2
^ permalink raw reply related
* [net-next 2/7] ixgb: Omit private ndo_get_stats function
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Tobias Klauser, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Tobias Klauser <tklauser@distanz.ch>
ixgb_get_stats() just returns dev->stats so we can leave it
out altogether and let dev_get_stats() do the job.
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index fbd220d137b3..5a713199653c 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -86,7 +86,6 @@ static void ixgb_set_multi(struct net_device *netdev);
static void ixgb_watchdog(unsigned long data);
static netdev_tx_t ixgb_xmit_frame(struct sk_buff *skb,
struct net_device *netdev);
-static struct net_device_stats *ixgb_get_stats(struct net_device *netdev);
static int ixgb_change_mtu(struct net_device *netdev, int new_mtu);
static int ixgb_set_mac(struct net_device *netdev, void *p);
static irqreturn_t ixgb_intr(int irq, void *data);
@@ -367,7 +366,6 @@ static const struct net_device_ops ixgb_netdev_ops = {
.ndo_open = ixgb_open,
.ndo_stop = ixgb_close,
.ndo_start_xmit = ixgb_xmit_frame,
- .ndo_get_stats = ixgb_get_stats,
.ndo_set_rx_mode = ixgb_set_multi,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = ixgb_set_mac,
@@ -1597,20 +1595,6 @@ ixgb_tx_timeout_task(struct work_struct *work)
}
/**
- * ixgb_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the timer callback.
- **/
-
-static struct net_device_stats *
-ixgb_get_stats(struct net_device *netdev)
-{
- return &netdev->stats;
-}
-
-/**
* ixgb_change_mtu - Change the Maximum Transfer Unit
* @netdev: network interface device structure
* @new_mtu: new value for maximum frame size
--
2.12.2
^ permalink raw reply related
* [net-next 3/7] e1000e: fix timing for 82579 Gigabit Ethernet controller
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Bernd Faust, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420233335.34900-1-jeffrey.t.kirsher@intel.com>
From: Bernd Faust <berndfaust@gmail.com>
After an upgrade to Linux kernel v4.x the hardware timestamps of the
82579 Gigabit Ethernet Controller are different than expected.
The values that are being read are almost four times as big as before
the kernel upgrade.
The difference is that after the upgrade the driver sets the clock
frequency to 25MHz, where before the upgrade it was set to 96MHz. Intel
confirmed that the correct frequency for this network adapter is 96MHz.
Signed-off-by: Bernd Faust <berndfaust@gmail.com>
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e9af89ad039c..667fc45ce906 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3511,6 +3511,12 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca)
switch (hw->mac.type) {
case e1000_pch2lan:
+ /* Stable 96MHz frequency */
+ incperiod = INCPERIOD_96MHz;
+ incvalue = INCVALUE_96MHz;
+ shift = INCVALUE_SHIFT_96MHz;
+ adapter->cc.shift = shift + INCPERIOD_SHIFT_96MHz;
+ break;
case e1000_pch_lpt:
if (er32(TSYNCRXCTL) & E1000_TSYNCRXCTL_SYSCFI) {
/* Stable 96MHz frequency */
--
2.12.2
^ permalink raw reply related
* [net-next 0/7][pull request] 1GbE Intel Wired LAN Driver Updates 2017-04-20
From: Jeff Kirsher @ 2017-04-20 23:33 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to e1000, e1000e, igb/vf and ixgb.
Tobias Klauser cleans up e1000, ixgb and igbvf from having a local
function or structure for netdev stats.
Bernd Faust fixes an issue for 82579 devices, where the clock frequency
was being incorrectly set for these devices. These devices only support
96MHz, so make sure they are set to use only that.
Yury Kylulin extends the work Jake and Alex did for ixgbe in MAC filter
handling into the igb driver.
Kim Tatt Chuah enables igb to wake up by packet and to read the necessary
Wake Up Status (WUS) and Wake Up Packet Memory (WUPM) registers.
The following are changes since commit 6905e5a5c8d552ace1d65cacb43499a0eb1e0b89:
net/mlx5e: IPoIB, Fix error handling in mlx5_rdma_netdev_alloc()
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE
Bernd Faust (1):
e1000e: fix timing for 82579 Gigabit Ethernet controller
Kim Tatt Chuah (1):
igb: Enable reading of wake up packet
Tobias Klauser (3):
e1000: Omit private ndo_get_stats function
ixgb: Omit private ndo_get_stats function
igbvf: Use net_device_stats from struct net_device
Yury Kylulin (2):
igb: improve MAC filter handling
igb/igbvf: Add VF MAC filter request capabilities
drivers/net/ethernet/intel/e1000/e1000_main.c | 15 -
drivers/net/ethernet/intel/e1000e/netdev.c | 6 +
drivers/net/ethernet/intel/igb/e1000_defines.h | 21 ++
drivers/net/ethernet/intel/igb/e1000_mbx.h | 4 +
drivers/net/ethernet/intel/igb/igb.h | 23 ++
drivers/net/ethernet/intel/igb/igb_main.c | 429 ++++++++++++++++++++-----
drivers/net/ethernet/intel/igbvf/igbvf.h | 3 +-
drivers/net/ethernet/intel/igbvf/mbx.h | 4 +
drivers/net/ethernet/intel/igbvf/netdev.c | 70 ++--
drivers/net/ethernet/intel/igbvf/vf.c | 41 +++
drivers/net/ethernet/intel/igbvf/vf.h | 1 +
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 16 -
12 files changed, 499 insertions(+), 134 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: [PATCH] p54: Prevent from dereferencing null pointer when releasing SKB
From: Greg Rose @ 2017-04-20 23:25 UTC (permalink / raw)
To: Myungho Jung; +Cc: edumazet, netdev
In-Reply-To: <20170420232349.GA25174@fqdn.specialj.com>
On Thu, 2017-04-20 at 16:23 -0700, Myungho Jung wrote:
> On Thu, Apr 20, 2017 at 04:03:43PM -0700, Greg Rose wrote:
> > On Thu, 2017-04-20 at 11:25 -0700, Myungho Jung wrote:
> > > Added NULL check to make __dev_kfree_skb_irq consistent with kfree
> > > family of functions.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289
> > >
> > > Signed-off-by: Myungho Jung <mhjungk@gmail.com>
> >
> > Hi,
> >
> > I think the patch is fine but I'm confused by the subject. You mention
> > p54 driver but the change is in dev.c. I know the bugzilla references
> > the p54 but that's not where the change is.
> >
> > Seems odd to me.
> >
> > - Greg
> >
> > > ---
> > > net/core/dev.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 7869ae3..22be2a6 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum skb_free_reason reason)
> > > {
> > > unsigned long flags;
> > >
> > > + if (unlikely(!skb))
> > > + return;
> > > +
> > > if (likely(atomic_read(&skb->users) == 1)) {
> > > smp_rmb();
> > > atomic_set(&skb->users, 0);
> >
> >
> >
>
> Hi Greg,
>
> Thank you for checking my patch. I missed that I moved change from p54
> to net/dev. Do I need to resubmit patch v2 to modify subject?
>
> Thanks,
> Myungho
Yes, I think that's the best idea.
Thanks,
- Greg
^ permalink raw reply
* Re: [PATCH] p54: Prevent from dereferencing null pointer when releasing SKB
From: Myungho Jung @ 2017-04-20 23:23 UTC (permalink / raw)
To: Greg Rose; +Cc: edumazet, netdev
In-Reply-To: <1492729423.17866.6.camel@gmail.com>
On Thu, Apr 20, 2017 at 04:03:43PM -0700, Greg Rose wrote:
> On Thu, 2017-04-20 at 11:25 -0700, Myungho Jung wrote:
> > Added NULL check to make __dev_kfree_skb_irq consistent with kfree
> > family of functions.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289
> >
> > Signed-off-by: Myungho Jung <mhjungk@gmail.com>
>
> Hi,
>
> I think the patch is fine but I'm confused by the subject. You mention
> p54 driver but the change is in dev.c. I know the bugzilla references
> the p54 but that's not where the change is.
>
> Seems odd to me.
>
> - Greg
>
> > ---
> > net/core/dev.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 7869ae3..22be2a6 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum skb_free_reason reason)
> > {
> > unsigned long flags;
> >
> > + if (unlikely(!skb))
> > + return;
> > +
> > if (likely(atomic_read(&skb->users) == 1)) {
> > smp_rmb();
> > atomic_set(&skb->users, 0);
>
>
>
Hi Greg,
Thank you for checking my patch. I missed that I moved change from p54
to net/dev. Do I need to resubmit patch v2 to modify subject?
Thanks,
Myungho
^ permalink raw reply
* Re: [PATCH V2 net] netdevice: Include NETIF_F_HW_CSUM when intersecting features
From: Vlad Yasevich @ 2017-04-20 23:19 UTC (permalink / raw)
To: Alexander Duyck, Vladislav Yasevich; +Cc: Netdev, Michal Kubecek, Tom Herbert
In-Reply-To: <CAKgT0UcHtWYQDcYda+i8R8WhBKeWbOBZRHDcAmJXZYuP1RM7-Q@mail.gmail.com>
On 04/20/2017 06:31 PM, Alexander Duyck wrote:
> On Thu, Apr 20, 2017 at 12:17 PM, Vladislav Yasevich
> <vyasevich@gmail.com> wrote:
>> While hardware device use either NETIF_F_(IP|IPV6)_CSUM or
>> NETIF_F_HW_CSUM, all of the software devices use HW_CSUM.
>> This results in an interesting situation when the software
>> device is configured on top of hw device using (IP|IPV6)_CSUM.
>> In this situation, the user can't turn off checksum offloading
>> features on the software device.
>
> Why wouldn't they be able to? It seems like the software device
> shouldn't be setting IP_CSUM or IPV6_CSUM feature flags, but they will
> be set in the intersect features. The result won't have
> NETIF_F_HW_CSUM set, but it should advertise the features of the lower
> device instead.
The can't because the upper software devices typically has HW_CSUM set in
hw_features and features. When we intersect with a lower device which has
IP|IPV6 set, we end up with a software device that has IP|IPV6 set. However,
the the hw_features have HW_CSUM, you end with a "fixed" setting of IP|IPV6
which can't be controlled now on the software device.
I've had a situation where trying to debug something, to turn off checksum
offloading on a vlan, I had to turn it of on the hw itself which effects all
traffic now.
We should be able to control it properly.
>
>> This patch resolves that by adding NETIF_F_HW_CSUM to the mask
>> if a feature set includes only IP|IPV6 csum. This allows the user
>> to control the upper (software) device checksum, while at the same
>> time correctly propagating lower device changes up.
>
> You can't go this way. HW_CSUM is an all inclusive feature flag,
> whereas IP_CSUM and IPV6_CSUM specify only specific offload types.
> With your change the lower device could disable IPV6_CSUM for instance
> but you would still end up advertising checksum offload via HW_CSUM.
>
>> CC: Michal Kubecek <mkubecek@suse.cz>
>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>> CC: Tom Herbert <tom@herbertland.com>
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>
>> ---
>>
>> V2: Addressed comments from Alex Duyck. I tested this with hacked virtio
>> device that set IP|IPV6 checksums instead of HW. Configuring a vlan on
>> top gave the vlan device with 'ip-generic: on' setting (using HW checksum).
>> This allows me to change vlan checksum offloads independent of virt-io nic.
>> Changes to virtio-nic propagated up to vlan, turning off the offloading
>> correctly.
>>
>> include/linux/netdevice.h | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index b0aa089..81aed2f 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -4009,10 +4009,10 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
>> netdev_features_t f2)
>> {
>> if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
>> - if (f1 & NETIF_F_HW_CSUM)
>> - f1 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
>> - else
>> - f2 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
>> + if (f1 & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))
>> + f1 |= NETIF_F_HW_CSUM;
>> + if(f2 & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))
>> + f2 |= NETIF_F_HW_CSUM;
>> }
>>
>> return f1 & f2;
>> --
>> 2.7.4
>>
>
> This doesn't work. "NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM" doesn't
> equate to "NETIF_F_HW_CSUM". The problem is NETIF_F_HW_CSUM is a
> catch-all, the IP and IPV6 CSUM flags are not. Right now you would
> introduce escapes on devices that enable IP but not IPv6, or the other
> way around.
>
> Can you point to the exact case where this code is an issue? It seems
> like maybe you are wanting to have NETIF_F_HW_CSUM set if you support
> offloading a given protocol. You might want to look at how we dealt
> with this in the GSO code path so that if we could offload the
> checksum we set NETIF_F_HW_CSUM based on protocol and the CSUM offload
> feature bit for that protocol.
>
What I'd like to be able to do is control features on the software device
without having to turn them off on the HW. As it stands right now, we can't
do that. If you want to try, configure a vlan on top of any device that
sets IP|IPV6 csum features.
I was trying to fix it in the common place. It actually works correctly
since the software checksum gets computed correctly lower in the stack,
so there is no actual escape.
Having said that, the other alternative is to inherit hw_features from
lower devices. BTW, bonding I think has a similar "issue" you are
describing since it prefers HW_CSUM if any of the slaves have it set.
-vlad
> - Alex
>
^ permalink raw reply
* Re: more on FP operations
From: Alexei Starovoitov @ 2017-04-20 23:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, ast, borkmann, netdev
In-Reply-To: <58F92DAC.3010706@iogearbox.net>
On Thu, Apr 20, 2017 at 11:52:44PM +0200, Daniel Borkmann wrote:
> On 04/20/2017 08:06 PM, David Miller wrote:
> >
> >I'm running test_verifier for testing, and I notice in my JIT that a
> >32-bit move from the frame pointer (BPF_REG_10) ends up in the JIT.
> >
> >It is from this test:
> >
> > "unpriv: partial copy of pointer",
> > .insns = {
> > BPF_MOV32_REG(BPF_REG_1, BPF_REG_10),
> > BPF_MOV64_IMM(BPF_REG_0, 0),
> > BPF_EXIT_INSN(),
> > },
> > .errstr_unpriv = "R10 partial copy",
> > .result_unpriv = REJECT,
> > .result = ACCEPT,
> >
> >It seems to suggest that privileged code is allowed to do this, but I
> >can't think of a legitimate usage.
>
> One thing I could think of right now would be for use in 32 bit
> archs, but that would still need to be taught to the verifier
> first. Other patterns f.e. like ...
>
> {
> "unpriv: adding of fp",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_1, 0),
> BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_10),
> BPF_MOV64_IMM(BPF_REG_0, 0),
> BPF_EXIT_INSN(),
> },
> .errstr_unpriv = "pointer arithmetic prohibited",
> .result_unpriv = REJECT,
> .result = ACCEPT,
> },
>
> ... are currently also possible, but in the above and the partial
> copy r1 is always considered as UNKNOWN_VALUE from that point onward
> and there's not really much we could do with it anymore, except
> perhaps passing to bpf_probe_read() for inspection in tracing for
> some reason. Since there are also various other pointers, it is
> really only the FP that needs to be special cased for sparc JIT,
> right?
Just remembered another issue with frame pointers.
In is_spillable_regtype() we have:
case PTR_TO_STACK:
...
case FRAME_PTR:
return true;
so both FP and FP-10 are spillable, since LLVM sometimes
thinks that it's cheaper to load stack pointer from stack memory
instead of recomputing it. It's doing it rarely. Unfortunately
I couldn't figure out how to teach llvm not to do that.
> >I really want to be able to JIT anything the verifier accepts, but I
> >have a hard time justifying adding 32-bit FP register move support,
> >adjusting for the stack bias, etc.
32-bit FP reg move is indeed useless.
The only reason I used R10 when i did that commit bf5088773faff
is because on the program entry it only has two valid pointers r1 and r10.
r1 was used in the other test and this test used r10.
If we only disallow 32-bit move from reg_type=FRAME_PTR
it's not going to be enough. We'd need to disallow spilling of
reg_type=FRAME_PTR, which I'm afraid may break some programs.
Like:
r2 = r10
*(u64*)stack = r2;
..stuff...
r3 = *(u64*)stack;
r3 += -10;
is currently recognized by the verifier, since it tracks pointer types
through spill/fill.
I made llvm never generate 'r3 -= 10' in the above example, but
I couldn't make it avoid spill/fill of pointers to stack.
It was trivial to support it in the verifier, since it has to track
spill/fill of other pointer types anyway. I didn't expect it to
cause JIT headaches.
Such spill/fill is rare, so if it's too ugly to support in sparc JIT
we can add a boolean flag where verifier will say that FP spill/fill
is used by the prog and JIT can ignore such progs.
I don't think JIT can detect that by itself and verifier help is needed.
^ permalink raw reply
* Re: [PATCH] p54: Prevent from dereferencing null pointer when releasing SKB
From: Greg Rose @ 2017-04-20 23:03 UTC (permalink / raw)
To: Myungho Jung; +Cc: edumazet, netdev
In-Reply-To: <1492712723-9350-1-git-send-email-mhjungk@gmail.com>
On Thu, 2017-04-20 at 11:25 -0700, Myungho Jung wrote:
> Added NULL check to make __dev_kfree_skb_irq consistent with kfree
> family of functions.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289
>
> Signed-off-by: Myungho Jung <mhjungk@gmail.com>
Hi,
I think the patch is fine but I'm confused by the subject. You mention
p54 driver but the change is in dev.c. I know the bugzilla references
the p54 but that's not where the change is.
Seems odd to me.
- Greg
> ---
> net/core/dev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7869ae3..22be2a6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum skb_free_reason reason)
> {
> unsigned long flags;
>
> + if (unlikely(!skb))
> + return;
> +
> if (likely(atomic_read(&skb->users) == 1)) {
> smp_rmb();
> atomic_set(&skb->users, 0);
^ permalink raw reply
* [PATCH v2] netpoll: Check for skb->queue_mapping
From: Tushar Dave @ 2017-04-20 22:57 UTC (permalink / raw)
To: davem, brouer, edumazet, netdev; +Cc: sowmini.varadhan
Reducing real_num_tx_queues needs to be in sync with skb queue_mapping
otherwise skbs with queue_mapping greater than real_num_tx_queues
can be sent to the underlying driver and can result in kernel panic.
One such event is running netconsole and enabling VF on the same
device. Or running netconsole and changing number of tx queues via
ethtool on same device.
e.g.
Unable to handle kernel NULL pointer dereference
tsk->{mm,active_mm}->context = 0000000000001525
tsk->{mm,active_mm}->pgd = fff800130ff9a000
\|/ ____ \|/
"@'/ .. \`@"
/_| \__/ |_\
\__U_/
kworker/48:1(475): Oops [#1]
CPU: 48 PID: 475 Comm: kworker/48:1 Tainted: G OE
4.11.0-rc3-davem-net+ #7
Workqueue: events queue_process
task: fff80013113299c0 task.stack: fff800131132c000
TSTATE: 0000004480e01600 TPC: 00000000103f9e3c TNPC: 00000000103f9e40 Y:
00000000 Tainted: G OE
TPC: <ixgbe_xmit_frame_ring+0x7c/0x6c0 [ixgbe]>
g0: 0000000000000000 g1: 0000000000003fff g2: 0000000000000000 g3:
0000000000000001
g4: fff80013113299c0 g5: fff8001fa6808000 g6: fff800131132c000 g7:
00000000000000c0
o0: fff8001fa760c460 o1: fff8001311329a50 o2: fff8001fa7607504 o3:
0000000000000003
o4: fff8001f96e63a40 o5: fff8001311d77ec0 sp: fff800131132f0e1 ret_pc:
000000000049ed94
RPC: <set_next_entity+0x34/0xb80>
l0: 0000000000000000 l1: 0000000000000800 l2: 0000000000000000 l3:
0000000000000000
l4: 000b2aa30e34b10d l5: 0000000000000000 l6: 0000000000000000 l7:
fff8001fa7605028
i0: fff80013111a8a00 i1: fff80013155a0780 i2: 0000000000000000 i3:
0000000000000000
i4: 0000000000000000 i5: 0000000000100000 i6: fff800131132f1a1 i7:
00000000103fa4b0
I7: <ixgbe_xmit_frame+0x30/0xa0 [ixgbe]>
Call Trace:
[00000000103fa4b0] ixgbe_xmit_frame+0x30/0xa0 [ixgbe]
[0000000000998c74] netpoll_start_xmit+0xf4/0x200
[0000000000998e10] queue_process+0x90/0x160
[0000000000485fa8] process_one_work+0x188/0x480
[0000000000486410] worker_thread+0x170/0x4c0
[000000000048c6b8] kthread+0xd8/0x120
[0000000000406064] ret_from_fork+0x1c/0x2c
[0000000000000000] (null)
Disabling lock debugging due to kernel taint
Caller[00000000103fa4b0]: ixgbe_xmit_frame+0x30/0xa0 [ixgbe]
Caller[0000000000998c74]: netpoll_start_xmit+0xf4/0x200
Caller[0000000000998e10]: queue_process+0x90/0x160
Caller[0000000000485fa8]: process_one_work+0x188/0x480
Caller[0000000000486410]: worker_thread+0x170/0x4c0
Caller[000000000048c6b8]: kthread+0xd8/0x120
Caller[0000000000406064]: ret_from_fork+0x1c/0x2c
Caller[0000000000000000]: (null)
Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
---
v1->v2:
- addressed comments from Eric Dumazet.
net/core/netpoll.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 9424673..29be246 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -105,15 +105,21 @@ static void queue_process(struct work_struct *work)
while ((skb = skb_dequeue(&npinfo->txq))) {
struct net_device *dev = skb->dev;
struct netdev_queue *txq;
+ unsigned int q_index;
if (!netif_device_present(dev) || !netif_running(dev)) {
kfree_skb(skb);
continue;
}
- txq = skb_get_tx_queue(dev, skb);
-
local_irq_save(flags);
+ /* check if skb->queue_mapping is still valid */
+ q_index = skb_get_queue_mapping(skb);
+ if (unlikely(q_index >= dev->real_num_tx_queues)) {
+ q_index = q_index % dev->real_num_tx_queues;
+ skb_set_queue_mapping(skb, q_index);
+ }
+ txq = netdev_get_tx_queue(dev, q_index);
HARD_TX_LOCK(dev, txq, smp_processor_id());
if (netif_xmit_frozen_or_stopped(txq) ||
netpoll_start_xmit(skb, dev, txq) != NETDEV_TX_OK) {
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] Fix values type used in test_maps
From: Alexei Starovoitov @ 2017-04-20 22:53 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, ast, borkmann, netdev
In-Reply-To: <58F92725.5050304@iogearbox.net>
On Thu, Apr 20, 2017 at 11:24:53PM +0200, Daniel Borkmann wrote:
> On 04/20/2017 09:20 PM, David Miller wrote:
> >
> >Maps of per-cpu type have their value element size adjusted to 8 if it
> >is specified smaller during various map operations.
> >
> >This makes test_maps as a 32-bit binary fail, in fact the kernel
> >writes past the end of the value's array on the user's stack.
> >
> >To be quite honest, I think the kernel should reject creation of a
> >per-cpu map that doesn't have a value size of at least 8 if that's
> >what the kernel is going to silently adjust to later.
>
> It's unintuitive, agree, and it's in fact a round_up(value_size, 8),
> so rejecting a value size smaller than 8 doesn't really help; commit
> 15a07b33814d ("bpf: add lookup/update support for per-cpu hash and
> array maps") explained the rationale a bit. Hmm, we should probably
> move at least the bpf_num_possible_cpus() and round_up(val_size, 8)
> calculation as a single wrapper function to be used for determining
> the array size into bpf_util.h, so that it's slightly easier to deal
> with.
yes.
The reason even non-percpu array does round_up(value_size, 8) is
to make sure that all elements are aligned, so when bpf prog does
struct foo *value = bpf_map_lookup(key)
..value->field.. // here the verifier can check alignment of ld/st properly
The reason we don't reject array value_size < 8 is to allow less
than 64-bit counters. Like this set:
struct array_value {
__u32 cnt1;
__u32 cnt2;
__u32 cnt3;
};
is valid and from bpf program only these 3 counters are accessible.
The kernel will allocate 16-byte for it and will copy it to user space
via bpf_long_memcpy(), since kernel doesn't know the contents of
the hash/array values.
I agree that is non intuitive and we need a helper in bpf_util.h
to let user space do 'char buf[round_up(value_size, 8)][nr_cpus]' cleanly.
> >If the user passed something smaller, it is a sizeof() calcualtion
> >based upon the type they will actually use (just like in this testcase
> >code) in later calls to the map operations.
>
> Fixes: df570f577231 ("samples/bpf: unit test for BPF_MAP_TYPE_PERCPU_ARRAY")
>
> >Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
For this test it's indeed a good fix.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* [PATCH net-next] net: dsa: Remove redundant NULL dst check
From: Florian Fainelli @ 2017-04-20 22:47 UTC (permalink / raw)
To: netdev
Cc: jbe, Florian Fainelli, Andrew Lunn, Vivien Didelot,
David S. Miller, open list
tag_lan9303.c does check for a NULL dst but that's already checked by
dsa_switch_rcv() one layer above.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/tag_lan9303.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 563b6c8fe445..70130ed5c21a 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -79,11 +79,6 @@ static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
struct dsa_switch *ds;
unsigned int source_port;
- if (unlikely(!dst)) {
- dev_warn_ratelimited(&dev->dev, "Dropping packet, due to missing switch tree device\n");
- return NULL;
- }
-
ds = dst->ds[0];
if (unlikely(!ds)) {
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net] net: ipv6: RTF_PCPU should not be settable from userspace
From: David Ahern @ 2017-04-20 22:43 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Martin KaFai Lau,
Andrey Konovalov
In-Reply-To: <CAM_iQpUbuzie86Cf96Q9F53UiamJixr-aVkAKtgvZFtiKPngUw@mail.gmail.com>
On 4/20/17 4:39 PM, Cong Wang wrote:
> On Wed, Apr 19, 2017 at 2:19 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>>
>> Fix by checking for the flag and failing with EINVAL.
>>
>
> I am still not sure about this. There are a few unused bits in
> this flag, we simply ignore the rest, right? Why should we
> reject this one instead of all of those we don't use?
>
RTF_PCPU most definitely should not be set by userspace. arguably it
should not be returned to userspace either, but it is part of the uapi.
I scanned the others. It is not clear that others should fail with
EINVAL. Certainly a mask of unused flags can be added, but to me that is
on top of this bug fix.
^ permalink raw reply
* Re: [PATCH net] net: ipv6: RTF_PCPU should not be settable from userspace
From: Cong Wang @ 2017-04-20 22:39 UTC (permalink / raw)
To: David Ahern
Cc: Linux Kernel Network Developers, Martin KaFai Lau,
Andrey Konovalov
In-Reply-To: <1492636783-29756-1-git-send-email-dsa@cumulusnetworks.com>
On Wed, Apr 19, 2017 at 2:19 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>
> Fix by checking for the flag and failing with EINVAL.
>
I am still not sure about this. There are a few unused bits in
this flag, we simply ignore the rest, right? Why should we
reject this one instead of all of those we don't use?
^ permalink raw reply
* Re: [PATCH net-next v8 2/3] Add a eBPF helper function to retrieve socket uid
From: Mickaël Salaün @ 2017-04-20 22:39 UTC (permalink / raw)
To: Chenbo Feng, netdev-u79uwXL29TY76Z2rM5mHXA, David Miller,
Alexei Starovoitov, Daniel Borkmann
Cc: Lorenzo Colitti, Willem de Bruijn, Chenbo Feng, Linux API
In-Reply-To: <1490228856-4546-3-git-send-email-chenbofeng.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3852 bytes --]
On 23/03/2017 01:27, Chenbo Feng wrote:
> From: Chenbo Feng <fengc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> Returns the owner uid of the socket inside a sk_buff. This is useful to
> perform per-UID accounting of network traffic or per-UID packet
> filtering. The socket need to be a fullsock otherwise overflowuid is
> returned.
>
> Signed-off-by: Chenbo Feng <fengc-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> include/uapi/linux/bpf.h | 9 ++++++++-
> net/core/filter.c | 22 ++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 3 ++-
> 3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index dc81a9f..ff42111 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -462,6 +462,12 @@ union bpf_attr {
> * @skb: pointer to skb
> * Return: 8 Bytes non-decreasing number on success or 0 if the socket
> * field is missing inside sk_buff
> + *
> + * u32 bpf_get_socket_uid(skb)
> + * Get the owner uid of the socket stored inside sk_buff.
> + * @skb: pointer to skb
> + * Return: uid of the socket owner on success or 0 if the socket pointer
> + * inside sk_buff is NULL
What about UID 0? Why not returning -1 on error?
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -510,7 +516,8 @@ union bpf_attr {
> FN(skb_change_head), \
> FN(xdp_adjust_head), \
> FN(probe_read_str), \
> - FN(get_socket_cookie),
> + FN(get_socket_cookie), \
> + FN(get_socket_uid),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5b65ae3..93cc4af 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2612,6 +2612,24 @@ static const struct bpf_func_proto bpf_get_socket_cookie_proto = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> +BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
> +{
> + struct sock *sk = sk_to_full_sk(skb->sk);
> + kuid_t kuid;
> +
> + if (!sk || !sk_fullsock(sk))
> + return overflowuid;
> + kuid = sock_net_uid(sock_net(sk), sk);
> + return from_kuid_munged(sock_net(sk)->user_ns, kuid);
> +}
> +
> +static const struct bpf_func_proto bpf_get_socket_uid_proto = {
> + .func = bpf_get_socket_uid,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> +};
> +
> static const struct bpf_func_proto *
> bpf_base_func_proto(enum bpf_func_id func_id)
> {
> @@ -2648,6 +2666,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
> return &bpf_skb_load_bytes_proto;
> case BPF_FUNC_get_socket_cookie:
> return &bpf_get_socket_cookie_proto;
> + case BPF_FUNC_get_socket_uid:
> + return &bpf_get_socket_uid_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -2709,6 +2729,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
> return &bpf_skb_under_cgroup_proto;
> case BPF_FUNC_get_socket_cookie:
> return &bpf_get_socket_cookie_proto;
> + case BPF_FUNC_get_socket_uid:
> + return &bpf_get_socket_uid_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index a94bdd3..4a2d56d 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -504,7 +504,8 @@ union bpf_attr {
> FN(skb_change_head), \
> FN(xdp_adjust_head), \
> FN(probe_read_str), \
> - FN(get_socket_cookie),
> + FN(get_socket_cookie), \
> + FN(get_socket_uid),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 0/3] l3mdev: Improve use with main table
From: David Ahern @ 2017-04-20 22:36 UTC (permalink / raw)
To: Robert Shearman, davem; +Cc: netdev
In-Reply-To: <1491834122-26252-1-git-send-email-rshearma@brocade.com>
On 4/10/17 8:21 AM, Robert Shearman wrote:
> Attempting to create a TCP socket not bound to a VRF device when a TCP
> socket bound to a VRF device with the same port exists (and vice
> versa) fails with EADDRINUSE. This limits the ability to use programs
> in selected mixed VRF/non-VRF contexts.
>
> This patch series solves the problem by extending the l3mdev be aware
> of the special semantics of the main table and fixing issues arising
> from the split local/main tables. A VRF master device created linking
> to the main table and used for these programs in the same way as those
> created for VRF tables can.
>
> Robert Shearman (3):
> ipv6: Fix route handling when using l3mdev set to main table
> ipv4: Fix route handling when using l3mdev set to main table
> l3mdev: Fix lookup in local table when using main table
>
> net/ipv4/af_inet.c | 4 +++-
> net/ipv4/fib_frontend.c | 14 +++++++++-----
> net/ipv4/raw.c | 5 ++++-
> net/ipv6/addrconf.c | 12 +++++++++---
> net/ipv6/route.c | 23 ++++++++++++++++++-----
> net/l3mdev/l3mdev.c | 26 ++++++++++++++++++++------
> 6 files changed, 63 insertions(+), 21 deletions(-)
>
With the change I mentioned earlier to fix the refcnt issue on top of
this patch set I see a number of failures:
- local IPv4 with 127.0.0.1 address - ping, tcp, udp tests fail
- all of the IPv4 multicast tests fail
- IPv6 linklocal and mcast addresses generally fail
- IPv6 global address on vrf device
^ permalink raw reply
* Re: [PATCH V2 net] netdevice: Include NETIF_F_HW_CSUM when intersecting features
From: Alexander Duyck @ 2017-04-20 22:31 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: Netdev, Vladislav Yasevich, Michal Kubecek, Tom Herbert
In-Reply-To: <1492715844-30273-1-git-send-email-vyasevic@redhat.com>
On Thu, Apr 20, 2017 at 12:17 PM, Vladislav Yasevich
<vyasevich@gmail.com> wrote:
> While hardware device use either NETIF_F_(IP|IPV6)_CSUM or
> NETIF_F_HW_CSUM, all of the software devices use HW_CSUM.
> This results in an interesting situation when the software
> device is configured on top of hw device using (IP|IPV6)_CSUM.
> In this situation, the user can't turn off checksum offloading
> features on the software device.
Why wouldn't they be able to? It seems like the software device
shouldn't be setting IP_CSUM or IPV6_CSUM feature flags, but they will
be set in the intersect features. The result won't have
NETIF_F_HW_CSUM set, but it should advertise the features of the lower
device instead.
> This patch resolves that by adding NETIF_F_HW_CSUM to the mask
> if a feature set includes only IP|IPV6 csum. This allows the user
> to control the upper (software) device checksum, while at the same
> time correctly propagating lower device changes up.
You can't go this way. HW_CSUM is an all inclusive feature flag,
whereas IP_CSUM and IPV6_CSUM specify only specific offload types.
With your change the lower device could disable IPV6_CSUM for instance
but you would still end up advertising checksum offload via HW_CSUM.
> CC: Michal Kubecek <mkubecek@suse.cz>
> CC: Alexander Duyck <alexander.duyck@gmail.com>
> CC: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>
> ---
>
> V2: Addressed comments from Alex Duyck. I tested this with hacked virtio
> device that set IP|IPV6 checksums instead of HW. Configuring a vlan on
> top gave the vlan device with 'ip-generic: on' setting (using HW checksum).
> This allows me to change vlan checksum offloads independent of virt-io nic.
> Changes to virtio-nic propagated up to vlan, turning off the offloading
> correctly.
>
> include/linux/netdevice.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index b0aa089..81aed2f 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4009,10 +4009,10 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
> netdev_features_t f2)
> {
> if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
> - if (f1 & NETIF_F_HW_CSUM)
> - f1 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> - else
> - f2 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> + if (f1 & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))
> + f1 |= NETIF_F_HW_CSUM;
> + if(f2 & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))
> + f2 |= NETIF_F_HW_CSUM;
> }
>
> return f1 & f2;
> --
> 2.7.4
>
This doesn't work. "NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM" doesn't
equate to "NETIF_F_HW_CSUM". The problem is NETIF_F_HW_CSUM is a
catch-all, the IP and IPV6 CSUM flags are not. Right now you would
introduce escapes on devices that enable IP but not IPv6, or the other
way around.
Can you point to the exact case where this code is an issue? It seems
like maybe you are wanting to have NETIF_F_HW_CSUM set if you support
offloading a given protocol. You might want to look at how we dealt
with this in the GSO code path so that if we could offload the
checksum we set NETIF_F_HW_CSUM based on protocol and the CSUM offload
feature bit for that protocol.
- Alex
^ permalink raw reply
* RE: [net-next 04/14] i40e: dump VF information in debugfs
From: Williams, Mitch A @ 2017-04-20 22:25 UTC (permalink / raw)
To: 'David Miller', Yuval.Mintz@cavium.com
Cc: gerlitz.or@gmail.com, Kirsher, Jeffrey T, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <20170420.112021.167483274923043370.davem@davemloft.net>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of David Miller
> Sent: Thursday, April 20, 2017 8:20 AM
> To: Yuval.Mintz@cavium.com
> Cc: gerlitz.or@gmail.com; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> Williams, Mitch A <mitch.a.williams@intel.com>; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 04/14] i40e: dump VF information in debugfs
>
> From: "Mintz, Yuval" <Yuval.Mintz@cavium.com>
> Date: Thu, 20 Apr 2017 10:09:28 +0000
>
> >> > Dump some internal state about VFs through debugfs. This provides
> >> > information not available with 'ip link show'.
> >>
> >> such as?
> >>
> >> donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push
> >> debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch
> >> team even went further and deleted that portion of the mlxsw driver --
> all to
> >> all, I don't see much point for these type of changes, thoughts?
> >
> > Don't want to hikjack your thread, but continuing this topic -
> > Is there some flat-out disapproval for debugfs in net-next now?
> >
> > We're currently internally engaged with adding qed support for register
> dumps
> > [~equivalents for `ethtool -d' outputs] through debugfs, on behalf of
> storage
> > drivers [qedi/qedf] lacking the API for doing that.
>
> I really hate to see debugfs things in networking drivers. It's a
> complete cop out for doing things properly.
>
> I push back, but I can only fight too much. If people want to keep
> adding stupid poorly designed crap endlessly to their drivers there
> is only so much I can do...
Honestly, Dave, I try very hard not to put random crap into debugfs. I felt that there was enough utility here to send this one up.
We already have debugfs hooks upstream that allow us to dump information about a specific VSI. This just adds a tiny bit to that to allow us to link a VF with its specific VSI. It's the missing link, if you will.
I'm not adding backdoor hooks to do naughty things, I'm just (very slightly) enhancing what's already there.
This particular piece has been very useful for my debugging, and we have customers that have asked for it as well.
If you still want to reject this, I won't fight and cry and stomp my little feet. But I honestly think that this a) has genuine utility, b) is fairly device specific, and c) isn't crap.
-Mitch
^ permalink raw reply
* Re: [PATCH net] ipv4: Avoid caching dsts when lookup skipped nh oif check
From: David Ahern @ 2017-04-20 22:18 UTC (permalink / raw)
To: Robert Shearman, davem; +Cc: netdev
In-Reply-To: <1492693132-4708-1-git-send-email-rshearma@brocade.com>
On 4/20/17 6:58 AM, Robert Shearman wrote:
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index acd69cfe2951..f667783ffd19 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2125,6 +2125,14 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
> fi = NULL;
> }
>
> + /* If the flag to skip the nh oif check is set then the output
> + * device may not match the nh device, so cannot use or add to
> + * cache in that case.
> + */
> + if (unlikely(fl4->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF &&
> + FIB_RES_NH(*res).nh_dev != dev_out))
> + do_cache = false;
> +
> fnhe = NULL;
> do_cache &= fi != NULL;
> if (do_cache) {
>
I believe this is a better fix:
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 5e1e60546fce..fb74a16958af 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2407,7 +2407,7 @@ struct rtable *__ip_route_output_key_hash(struct
net *net, struct flowi4 *fl4,
}
/* L3 master device is the loopback for that domain */
- dev_out = l3mdev_master_dev_rcu(dev_out) ? :
net->loopback_dev;
+ dev_out = l3mdev_master_dev_rcu(FIB_RES_DEV(res)) ? :
net->loopback_dev;
fl4->flowi4_oif = dev_out->ifindex;
flags |= RTCF_LOCAL;
goto make_route;
Fixes: 5f02ce24c2696 ("net: l3mdev: Allow the l3mdev to be a loopback")
With your change above, references to vrf devices are still taken
(dev_out is the vrf device based on the flow struct) even though the
route's nexthop is in another domain. And the commit log should
reference the use case which is policy routing overriding the VRF rule.
^ permalink raw reply related
* Re: [PATCH v4 net-next RFC] net: Generic XDP
From: Andy Gospodarek @ 2017-04-20 22:09 UTC (permalink / raw)
To: David Miller; +Cc: alexei.starovoitov, michael.chan, netdev, xdp-newbies
In-Reply-To: <20170419.214049.2190624657363276447.davem@davemloft.net>
On Wed, Apr 19, 2017 at 09:40:49PM -0400, David Miller wrote:
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Wed, 19 Apr 2017 10:29:03 -0400
>
> > So I tried a variety of things and the simplest change on top of yours that
> > works well for xdp1, xdp2, and xdp_tx_iptunnel.
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index b3d3a6e..1bab3dc 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4316,11 +4316,11 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> >
> > off = xdp.data - orig_data;
> > if (off)
> > - __skb_push(skb, off);
> > + __skb_push(skb, -off);
>
> We have to handle both pushing and popping headers, so could you
> please test the snippet I asked you to try?
>
I will tomorrow or by Monday of next week.
I'm also going to hack^W write a quick test app to exercise it as well.
> > if (off > 0)
> > __skb_pull(skb, off);
> > else if (off < 0)
> > __skb_push(skb, -off);
>
> Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] bonding: use 'M' or 'G' based on the speed
From: Stephen Hemminger @ 2017-04-20 21:53 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: j.vosburgh, vfalico, andy, netdev
In-Reply-To: <1492679195-19938-1-git-send-email-yanjun.zhu@oracle.com>
On Thu, 20 Apr 2017 05:06:35 -0400
Zhu Yanjun <yanjun.zhu@oracle.com> wrote:
> If the speed of the slave netdev is more than 1000M,
> it is better to use 'G' instead of 'M'.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
> drivers/net/bonding/bond_main.c | 8 ++++++--
> drivers/net/bonding/bond_procfs.c | 6 +++++-
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index af9f0ce..1aad13d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2147,9 +2147,13 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_set_backup_slave(slave);
> }
>
> - netdev_info(bond->dev, "link status definitely up for interface %s, %u Mbps %s duplex\n",
> + netdev_info(bond->dev, "link status definitely up for interface %s, %u%sbps %s duplex\n",
> slave->dev->name,
> - slave->speed == SPEED_UNKNOWN ? 0 : slave->speed,
> + slave->speed == SPEED_UNKNOWN ? 0 :
> + (slave->speed > 1000 ?
> + slave->speed / 1000 : slave->speed),
> + slave->speed > 1000 ?
> + slave->speed % 1000 ? ".5 G" : " G" : " M",
> slave->duplex ? "full" : "half");
Or drop the message all together. This is just log noise in a production system.
>
> /* notify ad that the link status has changed */
> diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
> index f514fe5..4c31055 100644
> --- a/drivers/net/bonding/bond_procfs.c
> +++ b/drivers/net/bonding/bond_procfs.c
> @@ -173,7 +173,11 @@ static void bond_info_show_slave(struct seq_file *seq,
> if (slave->speed == SPEED_UNKNOWN)
> seq_printf(seq, "Speed: %s\n", "Unknown");
> else
> - seq_printf(seq, "Speed: %d Mbps\n", slave->speed);
> + seq_printf(seq, "Speed: %d%sbps\n",
> + slave->speed > 1000 ?
> + slave->speed / 1000 : slave->speed,
> + slave->speed > 1000 ?
> + (slave->speed % 1000 ? ".5 G" : " G") : " M");
>
> if (slave->duplex == DUPLEX_UNKNOWN)
> seq_printf(seq, "Duplex: %s\n", "Unknown");
You can't change output formats of /proc since it technically and kernel/userspace ABI.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox