* [PATCH 04/14] net: pch_gbe: Remove sw_reset_phy HAL abstraction
From: Paul Burton @ 2018-06-23 3:17 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>
For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.
This patch removes the sw_reset_phy abstraction, which it turns out is
never even used. Its one implementation, which is already called
directly within the same translation unit, can therefore be made static
and removed from the pch_gbe_phy.h header.
Signed-off-by: Paul Burton <paul.burton@mips.com>
---
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 2 --
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 16 ----------------
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h | 1 -
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c | 2 +-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h | 1 -
5 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 5dbfcd55efa8..47ee7428c3d3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -335,7 +335,6 @@ struct pch_gbe_hw;
* @read_phy_reg: for pch_gbe_hal_read_phy_reg
* @write_phy_reg: for pch_gbe_hal_write_phy_reg
* @reset_phy: for pch_gbe_hal_phy_hw_reset
- * @sw_reset_phy: for pch_gbe_hal_phy_sw_reset
*/
struct pch_gbe_functions {
void (*get_bus_info) (struct pch_gbe_hw *);
@@ -343,7 +342,6 @@ struct pch_gbe_functions {
s32 (*read_phy_reg) (struct pch_gbe_hw *, u32, u16 *);
s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
void (*reset_phy) (struct pch_gbe_hw *);
- void (*sw_reset_phy) (struct pch_gbe_hw *);
};
/**
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 3c6e009955ab..e1ecfb076029 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -89,7 +89,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
.read_phy_reg = pch_gbe_phy_read_reg_miic,
.write_phy_reg = pch_gbe_phy_write_reg_miic,
.reset_phy = pch_gbe_phy_hw_reset,
- .sw_reset_phy = pch_gbe_phy_sw_reset,
};
/**
@@ -204,18 +203,3 @@ void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw)
}
hw->func->reset_phy(hw);
}
-
-/**
- * pch_gbe_hal_phy_sw_reset - Soft PHY reset
- * @hw: Pointer to the HW structure
- */
-void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw)
-{
- if (!hw->func->sw_reset_phy) {
- struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
- netdev_err(adapter->netdev, "ERROR: configuration\n");
- return;
- }
- hw->func->sw_reset_phy(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 13fcdfb4a94d..aa802f670055 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -27,6 +27,5 @@ s32 pch_gbe_hal_init_hw(struct pch_gbe_hw *hw);
s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
-void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
#endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index a5cad5ea9436..6b35b573beef 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -184,7 +184,7 @@ s32 pch_gbe_phy_write_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 data)
* pch_gbe_phy_sw_reset - PHY software reset
* @hw: Pointer to the HW structure
*/
-void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw)
+static void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw)
{
u16 phy_ctrl;
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index 95ad0151ad02..efb955be8cac 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -26,7 +26,6 @@
s32 pch_gbe_phy_get_id(struct pch_gbe_hw *hw);
s32 pch_gbe_phy_read_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 *data);
s32 pch_gbe_phy_write_reg_miic(struct pch_gbe_hw *hw, u32 offset, u16 data);
-void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw);
void pch_gbe_phy_hw_reset(struct pch_gbe_hw *hw);
void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
--
2.17.1
^ permalink raw reply related
* [PATCH 03/14] net: pch_gbe: Remove read_mac_addr HAL abstraction
From: Paul Burton @ 2018-06-23 3:17 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>
For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.
This patch removes the read_mac_addr abstraction in favor of calling
pch_gbe_mac_read_mac_addr directly. Since this is defined in the same
translation unit as all of its callers, we can make it static & remove
it from the pch_gbe.h header.
Signed-off-by: Paul Burton <paul.burton@mips.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 3 ---
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 19 -------------------
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.h | 1 -
.../ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 4 ++--
4 files changed, 2 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 8dc40faef720..5dbfcd55efa8 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -336,7 +336,6 @@ struct pch_gbe_hw;
* @write_phy_reg: for pch_gbe_hal_write_phy_reg
* @reset_phy: for pch_gbe_hal_phy_hw_reset
* @sw_reset_phy: for pch_gbe_hal_phy_sw_reset
- * @read_mac_addr: for pch_gbe_hal_read_mac_addr
*/
struct pch_gbe_functions {
void (*get_bus_info) (struct pch_gbe_hw *);
@@ -345,7 +344,6 @@ struct pch_gbe_functions {
s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
void (*reset_phy) (struct pch_gbe_hw *);
void (*sw_reset_phy) (struct pch_gbe_hw *);
- s32 (*read_mac_addr) (struct pch_gbe_hw *);
};
/**
@@ -676,7 +674,6 @@ void pch_gbe_set_ethtool_ops(struct net_device *netdev);
/* pch_gbe_mac.c */
s32 pch_gbe_mac_force_mac_fc(struct pch_gbe_hw *hw);
-s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw);
u16 pch_gbe_mac_ctrl_miim(struct pch_gbe_hw *hw, u32 addr, u32 dir, u32 reg,
u16 data);
#endif /* _PCH_GBE_H_ */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index d66933b68934..3c6e009955ab 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -90,7 +90,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
.write_phy_reg = pch_gbe_phy_write_reg_miic,
.reset_phy = pch_gbe_phy_hw_reset,
.sw_reset_phy = pch_gbe_phy_sw_reset,
- .read_mac_addr = pch_gbe_mac_read_mac_addr
};
/**
@@ -220,21 +219,3 @@ void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw)
}
hw->func->sw_reset_phy(hw);
}
-
-/**
- * pch_gbe_hal_read_mac_addr - Reads MAC address
- * @hw: Pointer to the HW structure
- * Returns:
- * 0: Successfully
- * ENOSYS: Function is not registered
- */
-s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw)
-{
- if (!hw->func->read_mac_addr) {
- struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
-
- netdev_err(adapter->netdev, "ERROR: configuration\n");
- return -ENOSYS;
- }
- return hw->func->read_mac_addr(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index be2f202c26c4..13fcdfb4a94d 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -28,6 +28,5 @@ s32 pch_gbe_hal_read_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 *data);
s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
-s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw);
#endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 13fc828c7fd3..fc5079fa01e8 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -287,7 +287,7 @@ static inline void pch_gbe_mac_load_mac_addr(struct pch_gbe_hw *hw)
* Returns:
* 0: Successful.
*/
-s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
+static s32 pch_gbe_mac_read_mac_addr(struct pch_gbe_hw *hw)
{
struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
u32 adr1a, adr1b;
@@ -2627,7 +2627,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
pch_gbe_hal_get_bus_info(&adapter->hw);
/* Read the MAC address. and store to the private data */
- ret = pch_gbe_hal_read_mac_addr(&adapter->hw);
+ ret = pch_gbe_mac_read_mac_addr(&adapter->hw);
if (ret) {
dev_err(&pdev->dev, "MAC address Read Error\n");
goto err_free_adapter;
--
2.17.1
^ permalink raw reply related
* [PATCH 02/14] net: pch_gbe: Remove power_{up,down}_phy HAL abstraction
From: Paul Burton @ 2018-06-23 3:17 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>
For some reason the pch_gbe driver contains a struct pch_gbe_functions
with pointers used by a HAL abstraction layer, even though there is only
one implementation of each function.
This patch removes the power_up_phy & power_down_phy abstractions in
favor of calling pch_phy_power_up & pch_phy_power_down directly.
Signed-off-by: Paul Burton <paul.burton@mips.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 4 ----
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 22 -------------------
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.h | 2 --
.../ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 12 +++++-----
4 files changed, 6 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 697e29dd4bd3..8dc40faef720 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -336,8 +336,6 @@ struct pch_gbe_hw;
* @write_phy_reg: for pch_gbe_hal_write_phy_reg
* @reset_phy: for pch_gbe_hal_phy_hw_reset
* @sw_reset_phy: for pch_gbe_hal_phy_sw_reset
- * @power_up_phy: for pch_gbe_hal_power_up_phy
- * @power_down_phy: for pch_gbe_hal_power_down_phy
* @read_mac_addr: for pch_gbe_hal_read_mac_addr
*/
struct pch_gbe_functions {
@@ -347,8 +345,6 @@ struct pch_gbe_functions {
s32 (*write_phy_reg) (struct pch_gbe_hw *, u32, u16);
void (*reset_phy) (struct pch_gbe_hw *);
void (*sw_reset_phy) (struct pch_gbe_hw *);
- void (*power_up_phy) (struct pch_gbe_hw *hw);
- void (*power_down_phy) (struct pch_gbe_hw *hw);
s32 (*read_mac_addr) (struct pch_gbe_hw *);
};
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index 51250363566b..d66933b68934 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -90,8 +90,6 @@ static const struct pch_gbe_functions pch_gbe_ops = {
.write_phy_reg = pch_gbe_phy_write_reg_miic,
.reset_phy = pch_gbe_phy_hw_reset,
.sw_reset_phy = pch_gbe_phy_sw_reset,
- .power_up_phy = pch_gbe_phy_power_up,
- .power_down_phy = pch_gbe_phy_power_down,
.read_mac_addr = pch_gbe_mac_read_mac_addr
};
@@ -240,23 +238,3 @@ s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw)
}
return hw->func->read_mac_addr(hw);
}
-
-/**
- * pch_gbe_hal_power_up_phy - Power up PHY
- * @hw: Pointer to the HW structure
- */
-void pch_gbe_hal_power_up_phy(struct pch_gbe_hw *hw)
-{
- if (hw->func->power_up_phy)
- hw->func->power_up_phy(hw);
-}
-
-/**
- * pch_gbe_hal_power_down_phy - Power down PHY
- * @hw: Pointer to the HW structure
- */
-void pch_gbe_hal_power_down_phy(struct pch_gbe_hw *hw)
-{
- if (hw->func->power_down_phy)
- hw->func->power_down_phy(hw);
-}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
index 91ce07c8306c..be2f202c26c4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
@@ -29,7 +29,5 @@ s32 pch_gbe_hal_write_phy_reg(struct pch_gbe_hw *hw, u32 offset, u16 data);
void pch_gbe_hal_phy_hw_reset(struct pch_gbe_hw *hw);
void pch_gbe_hal_phy_sw_reset(struct pch_gbe_hw *hw);
s32 pch_gbe_hal_read_mac_addr(struct pch_gbe_hw *hw);
-void pch_gbe_hal_power_up_phy(struct pch_gbe_hw *hw);
-void pch_gbe_hal_power_down_phy(struct pch_gbe_hw *hw);
#endif
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 044a7561752c..13fc828c7fd3 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2072,7 +2072,7 @@ static int pch_gbe_open(struct net_device *netdev)
err = pch_gbe_setup_rx_resources(adapter, adapter->rx_ring);
if (err)
goto err_setup_rx;
- pch_gbe_hal_power_up_phy(hw);
+ pch_gbe_phy_power_up(hw);
err = pch_gbe_up(adapter);
if (err)
goto err_up;
@@ -2081,7 +2081,7 @@ static int pch_gbe_open(struct net_device *netdev)
err_up:
if (!adapter->wake_up_evt)
- pch_gbe_hal_power_down_phy(hw);
+ pch_gbe_phy_power_down(hw);
pch_gbe_free_rx_resources(adapter, adapter->rx_ring);
err_setup_rx:
pch_gbe_free_tx_resources(adapter, adapter->tx_ring);
@@ -2104,7 +2104,7 @@ static int pch_gbe_stop(struct net_device *netdev)
pch_gbe_down(adapter);
if (!adapter->wake_up_evt)
- pch_gbe_hal_power_down_phy(hw);
+ pch_gbe_phy_power_down(hw);
pch_gbe_free_tx_resources(adapter, adapter->tx_ring);
pch_gbe_free_rx_resources(adapter, adapter->rx_ring);
return 0;
@@ -2434,7 +2434,7 @@ static pci_ers_result_t pch_gbe_io_slot_reset(struct pci_dev *pdev)
}
pci_set_master(pdev);
pci_enable_wake(pdev, PCI_D0, 0);
- pch_gbe_hal_power_up_phy(hw);
+ pch_gbe_phy_power_up(hw);
pch_gbe_reset(adapter);
/* Clear wake up status */
pch_gbe_mac_set_wol_event(hw, 0);
@@ -2479,7 +2479,7 @@ static int __pch_gbe_suspend(struct pci_dev *pdev)
pch_gbe_mac_set_wol_event(hw, wufc);
pci_disable_device(pdev);
} else {
- pch_gbe_hal_power_down_phy(hw);
+ pch_gbe_phy_power_down(hw);
pch_gbe_mac_set_wol_event(hw, wufc);
pci_disable_device(pdev);
}
@@ -2508,7 +2508,7 @@ static int pch_gbe_resume(struct device *device)
return err;
}
pci_set_master(pdev);
- pch_gbe_hal_power_up_phy(hw);
+ pch_gbe_phy_power_up(hw);
pch_gbe_reset(adapter);
/* Clear wake on lan control and status */
pch_gbe_mac_set_wol_event(hw, 0);
--
2.17.1
^ permalink raw reply related
* [PATCH 01/14] net: pch_gbe: Remove unused copybreak parameter
From: Paul Burton @ 2018-06-23 3:17 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
In-Reply-To: <20180623031753.31893-1-paul.burton@mips.com>
The pch_gbe driver includes a 'copybreak' parameter which appears to
have been copied from the e1000e driver but is entirely unused. Remove
the dead code.
Signed-off-by: Paul Burton <paul.burton@mips.com>
---
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 34a1581eda95..044a7561752c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -34,7 +34,6 @@ const char pch_driver_version[] = DRV_VERSION;
#define PCH_GBE_DMA_ALIGN 0
#define PCH_GBE_DMA_PADDING 2
#define PCH_GBE_WATCHDOG_PERIOD (5 * HZ) /* watchdog time */
-#define PCH_GBE_COPYBREAK_DEFAULT 256
#define PCH_GBE_PCI_BAR 1
#define PCH_GBE_RESERVE_MEMORY 0x200000 /* 2MB */
@@ -113,8 +112,6 @@ const char pch_driver_version[] = DRV_VERSION;
#define MINNOW_PHY_RESET_GPIO 13
-static unsigned int copybreak __read_mostly = PCH_GBE_COPYBREAK_DEFAULT;
-
static int pch_gbe_mdio_read(struct net_device *netdev, int addr, int reg);
static void pch_gbe_mdio_write(struct net_device *netdev, int addr, int reg,
int data);
@@ -2784,14 +2781,6 @@ static int __init pch_gbe_init_module(void)
pr_info("EG20T PCH Gigabit Ethernet Driver - version %s\n",DRV_VERSION);
ret = pci_register_driver(&pch_gbe_driver);
- if (copybreak != PCH_GBE_COPYBREAK_DEFAULT) {
- if (copybreak == 0) {
- pr_info("copybreak disabled\n");
- } else {
- pr_info("copybreak enabled for packets <= %u bytes\n",
- copybreak);
- }
- }
return ret;
}
@@ -2809,8 +2798,4 @@ MODULE_LICENSE("GPL");
MODULE_VERSION(DRV_VERSION);
MODULE_DEVICE_TABLE(pci, pch_gbe_pcidev_id);
-module_param(copybreak, uint, 0644);
-MODULE_PARM_DESC(copybreak,
- "Maximum size of packet that is copied to a new buffer on receive");
-
/* pch_gbe_main.c */
--
2.17.1
^ permalink raw reply related
* [PATCH 00/14] net: pch_gbe: Cleanups
From: Paul Burton @ 2018-06-23 3:17 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Paul Burton
This series begins the process of cleaning up the pch_gbe network
driver. Whilst my ultimate goal is to add support for using this driver
on the MIPS Boston development board, this series sets that aside in
favor of making some more general cleanups. My hope is that this will
both make the driver a little more maleable & reduce the probability of
me gouging out my eyes.
Applies cleanly atop net-next as of 5424ea27390f ("netns: get more
entropy from net_hash_mix()").
Thanks,
Paul
Paul Burton (14):
net: pch_gbe: Remove unused copybreak parameter
net: pch_gbe: Remove power_{up,down}_phy HAL abstraction
net: pch_gbe: Remove read_mac_addr HAL abstraction
net: pch_gbe: Remove sw_reset_phy HAL abstraction
net: pch_gbe: Remove reset_phy HAL abstraction
net: pch_gbe: Remove {read,write}_phy_reg HAL abstraction
net: pch_gbe: Remove init_hw HAL abstraction
net: pch_gbe: Remove get_bus_info HAL abstraction
net: pch_gbe: Remove pch_gbe_hal_setup_init_funcs
net: pch_gbe: Remove PCH_GBE_MAC_IFOP_RGMII define
net: pch_gbe: Remove dead RINGFREE code
net: pch_gbe: Use module_pci_driver()
net: pch_gbe: Inline pch_gbe_mac_mc_addr_list_update
net: pch_gbe: Clean up pch_gbe_set_multi
.../net/ethernet/oki-semi/pch_gbe/Makefile | 2 +-
.../net/ethernet/oki-semi/pch_gbe/pch_gbe.h | 40 ---
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.c | 262 ------------------
.../ethernet/oki-semi/pch_gbe/pch_gbe_api.h | 35 ---
.../oki-semi/pch_gbe/pch_gbe_ethtool.c | 19 +-
.../ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 193 ++++---------
.../ethernet/oki-semi/pch_gbe/pch_gbe_phy.c | 2 +-
.../ethernet/oki-semi/pch_gbe/pch_gbe_phy.h | 2 -
8 files changed, 66 insertions(+), 489 deletions(-)
delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
delete mode 100644 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.h
--
2.17.1
^ permalink raw reply
* Re: [PATCH net-next] netns: get more entropy from net_hash_mix()
From: David Miller @ 2018-06-23 2:00 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180622232747.145594-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 22 Jun 2018 16:27:47 -0700
> struct net are effectively allocated from order-1 pages on x86,
> with one object per slab, meaning that the 13 low order bits
> of their addresses are zero.
>
> Once shifted by L1_CACHE_SHIFT, this leaves 7 zero-bits,
> meaning that net_hash_mix() does not help spreading
> objects on various hash tables.
>
> For example, TCP listen table has 32 buckets, meaning that
> all netns use the same bucket for port 80 or port 443.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Maciej Żenczykowski <maze@google.com>
Good catch, applied, thanks Eric.
^ permalink raw reply
* Re: [Patch net] net_sched: remove a bogus warning in hfsc
From: David Miller @ 2018-06-23 1:59 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, pupilla
In-Reply-To: <20180622213316.8080-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 22 Jun 2018 14:33:16 -0700
> In update_vf():
>
> cftree_remove(cl);
> update_cfmin(cl->cl_parent);
>
> the cl_cfmin of cl->cl_parent is intentionally updated to 0
> when that parent only has one child. And if this parent is
> root qdisc, we could end up, in hfsc_schedule_watchdog(),
> that we can't decide the next schedule time for qdisc watchdog.
> But it seems safe that we can just skip it, as this watchdog is
> not always scheduled anyway.
>
> Thanks to Marco for testing all the cases, nothing is broken.
>
> Reported-by: Marco Berizzi <pupilla@libero.it>
> Tested-by: Marco Berizzi <pupilla@libero.it>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks Cong.
^ permalink raw reply
* Re: [PATCH] ipv6: avoid copy_from_user() via ipv6_renew_options_kern()
From: David Miller @ 2018-06-23 1:57 UTC (permalink / raw)
To: pmoore; +Cc: netdev, selinux, linux-security-module
In-Reply-To: <152970230022.7734.15824980755229329454.stgit@chester>
From: Paul Moore <pmoore@redhat.com>
Date: Fri, 22 Jun 2018 17:18:20 -0400
> - const mm_segment_t old_fs = get_fs();
> -
> - set_fs(KERNEL_DS);
> - ret_val = ipv6_renew_options(sk, opt, newtype,
> - (struct ipv6_opt_hdr __user *)newopt,
> - newoptlen);
> - set_fs(old_fs);
So is it really the case that the traditional construct:
set_fs(KERNEL_DS);
... copy_{from,to}_user(...);
set_fs(old_fs);
is no longer allowed?
Setting fs to KERNEL_DS is supposed to make user copies work on kernel
memory. Or at least it did for 20+ years :-)
^ permalink raw reply
* Re: [PATCH] net: drivers/net: Convert random_ether_addr to eth_random_addr
From: David Miller @ 2018-06-23 1:50 UTC (permalink / raw)
To: joe
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
ulli.kroll, linus.walleij, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, bryan.whitehead, UNGLinuxDriver, harish.patil,
manish.chopra, Dept-GELinuxNICDev, linux-net-drivers, ecree,
bkenward, grygorii.strashko, w-kwok2, m-karicheri2, jdmason,
dave.jiang, allenbh, woojung.huh, ath9k-devel, mareklindner, sw,
a, kvalo, netdev
In-Reply-To: <c558b59181756dfe647bcdeaf659c386701c0948.1529689773.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Fri, 22 Jun 2018 10:51:00 -0700
> random_ether_addr is a #define for eth_random_addr which is
> generally preferred in kernel code by ~3:1
>
> Convert the uses of random_ether_addr to enable removing the #define
>
> Miscellanea:
>
> o Convert &vfmac[0] to equivalent vfmac and avoid unnecessary line wrap
>
> Signed-off-by: Joe Perches <joe@perches.com>
I'll apply this to net-next, thanks Joe.
^ permalink raw reply
* Re: [PATCH net 0/2] net: dccp: fixes around rx_tstamp_last_feedback
From: David Miller @ 2018-06-23 1:47 UTC (permalink / raw)
To: edumazet; +Cc: netdev, gerrit, dccp, eric.dumazet
In-Reply-To: <20180622134415.104266-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 22 Jun 2018 06:44:13 -0700
> This patch series fix some issues with rx_tstamp_last_feedback.
>
> - Switch to monotonic clock.
> - Avoid potential overflows on fast hosts/networks.
Series applied and queued up for -stable.
Thank you.
^ permalink raw reply
* Re: [PATCH v4] MAINTAINERS: Add file patterns for dsa device tree bindings
From: David Miller @ 2018-06-23 1:45 UTC (permalink / raw)
To: geert
Cc: andrew, vivien.didelot, f.fainelli, robh+dt, mark.rutland, netdev,
devicetree, linux-kernel
In-Reply-To: <20180622100813.29122-1-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Fri, 22 Jun 2018 12:08:13 +0200
> Submitters of device tree binding documentation may forget to CC
> the subsystem maintainer if this is missing.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] net: mscc: make sparse happy
From: David Miller @ 2018-06-23 1:44 UTC (permalink / raw)
To: antoine.tenart
Cc: f.fainelli, andrew, netdev, linux-kernel, thomas.petazzoni,
alexandre.belloni, quentin.schulz, allan.nielsen
In-Reply-To: <20180622095052.26726-1-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Fri, 22 Jun 2018 11:50:52 +0200
> This patch fixes a sparse warning about using an incorrect type in
> argument 2 of ocelot_write_rix(), as an u32 was expected but a __be32
> was given. The conversion to u32 is forced, which is safe as the value
> will be written as-is in the hardware without any modification.
>
> Fixes: 08d02364b12f ("net: mscc: fix the injection header")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: mvneta: fix the Rx desc DMA address in the Rx path
From: David Miller @ 2018-06-23 1:41 UTC (permalink / raw)
To: antoine.tenart
Cc: gregory.clement, mw, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, miquel.raynal, nadavh, stefanc, ymarkman
In-Reply-To: <20180622081539.16584-1-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Fri, 22 Jun 2018 10:15:39 +0200
> When using s/w buffer management, buffers are allocated and DMA mapped.
> When doing so on an arm64 platform, an offset correction is applied on
> the DMA address, before storing it in an Rx descriptor. The issue is
> this DMA address is then used later in the Rx path without removing the
> offset correction. Thus the DMA address is wrong, which can led to
> various issues.
>
> This patch fixes this by removing the offset correction from the DMA
> address retrieved from the Rx descriptor before using it in the Rx path.
>
> Fixes: 8d5047cf9ca2 ("net: mvneta: Convert to be 64 bits compatible")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH 0/4] docs: e100[0] fix build errors
From: David Miller @ 2018-06-23 1:38 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: me, corbet, linux-doc, netdev, linux-kernel
In-Reply-To: <5718d34758dd55090dbc73441c3b12c6c5c02ff8.camel@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 22 Jun 2018 09:44:00 -0700
> On Fri, 2018-06-22 at 10:37 +1000, Tobin C. Harding wrote:
>> I may be a little confused here, I'm getting docs build failure on
>> Linus' mainline, linux-next, and your docs-next but different errors.
>> There seems to be patches to the first two trees that are not in your
>> docs-next tree?
>>
>> Do networking docs typically go through your tree? Looks like
>> networking has done some conversion to rst that hasn't gone through
>> your
>> tree. Or else my git skills are failing.
>
> Networking documentation changes went through David Miller's networking
> tree because he maintains changes under Documentation/networking/
I've applied this series of fixes to my 'net' tree, thanks everyone.
^ permalink raw reply
* Re: [PATCH] net: phy: Allow compile test of GPIO consumers if !GPIOLIB
From: David Miller @ 2018-06-23 1:34 UTC (permalink / raw)
To: geert; +Cc: andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <20180621185800.17371-1-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Thu, 21 Jun 2018 20:58:00 +0200
> The GPIO subsystem provides dummy GPIO consumer functions if GPIOLIB is
> not enabled. Hence drivers that depend on GPIOLIB, but use GPIO consumer
> functionality only, can still be compiled if GPIOLIB is not enabled.
>
> Relax the dependency on GPIOLIB if COMPILE_TEST is enabled, where
> appropriate.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Applied to net-next, thank you.
^ permalink raw reply
* Re: [PATCH net] ipv6: mcast: fix unsolicited report interval after receiving querys
From: David Miller @ 2018-06-23 1:28 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, hannes, xiyou.wangcong, yoshfuji
In-Reply-To: <1529581776-30260-1-git-send-email-liuhangbin@gmail.com>
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Thu, 21 Jun 2018 19:49:36 +0800
> After recieving MLD querys, we update idev->mc_maxdelay with max_delay
> from query header. This make the later unsolicited reports have the same
> interval with mc_maxdelay, which means we may send unsolicited reports with
> long interval time instead of default configured interval time.
>
> Also as we will not call ipv6_mc_reset() after device up. This issue will
> be there even after leave the group and join other groups.
>
> Fixes: fc4eba58b4c14 ("ipv6: make unsolicited report intervals configurable for mld")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH] fman: don't set node on dpaa-ethernet platform device
From: David Miller @ 2018-06-23 1:27 UTC (permalink / raw)
To: bas; +Cc: netdev, madalin.bucur, linux-kernel
In-Reply-To: <20180621114222.8326-1-bas@daedalean.ai>
From: Bas Vermeulen <bas@daedalean.ai>
Date: Thu, 21 Jun 2018 13:42:22 +0200
> Setting dev->node to the mac_node in dpaa_eth_add_device during probe
> causes the mac_probe to be called again for the dpaa-ethernet.* device
> that was just added.
>
> Fix this by not setting dev->node, as it is not needed.
>
> Signed-off-by: Bas Vermeulen <bas@daedalean.ai>
This patch doesn't apply to the current sources.
^ permalink raw reply
* Re: [PATCH net] vhost_net: validate sock before trying to put its fd
From: David Miller @ 2018-06-23 1:24 UTC (permalink / raw)
To: jasowang; +Cc: kvm, mst, netdev, linux-kernel, virtualization, dan.carpenter
In-Reply-To: <1529557891-4828-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 21 Jun 2018 13:11:31 +0800
> Sock will be NULL if we pass -1 to vhost_net_set_backend(), but when
> we meet errors during ubuf allocation, the code does not check for
> NULL before calling sockfd_put(), this will lead NULL
> dereferencing. Fixing by checking sock pointer before.
>
> Fixes: bab632d69ee4 ("vhost: vhost TX zero-copy support")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks Jason.
^ permalink raw reply
* [PATCH net-next 4/4] selftests: rtnetlink: add ipsec offload API test
From: Shannon Nelson @ 2018-06-23 0:31 UTC (permalink / raw)
To: davem, netdev, jakub.kicinski; +Cc: anders.roxell, linux-kselftest
In-Reply-To: <1529713898-9200-1-git-send-email-shannon.nelson@oracle.com>
Using the netdevsim as a device for testing, try out the XFRM commands
for setting up IPsec hardware offloads.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
tools/testing/selftests/net/rtnetlink.sh | 114 +++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 15948cf..9e1a82e 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -608,6 +608,119 @@ kci_test_ipsec()
echo "PASS: ipsec"
}
+#-------------------------------------------------------------------
+# Example commands
+# ip x s add proto esp src 14.0.0.52 dst 14.0.0.70 \
+# spi 0x07 mode transport reqid 0x07 replay-window 32 \
+# aead 'rfc4106(gcm(aes))' 1234567890123456dcba 128 \
+# sel src 14.0.0.52/24 dst 14.0.0.70/24
+# offload dev sim1 dir out
+# ip x p add dir out src 14.0.0.52/24 dst 14.0.0.70/24 \
+# tmpl proto esp src 14.0.0.52 dst 14.0.0.70 \
+# spi 0x07 mode transport reqid 0x07
+#
+#-------------------------------------------------------------------
+kci_test_ipsec_offload()
+{
+ ret=0
+ algo="aead rfc4106(gcm(aes)) 0x3132333435363738393031323334353664636261 128"
+ srcip=192.168.123.3
+ dstip=192.168.123.4
+ dev=simx1
+ sysfsd=/sys/kernel/debug/netdevsim/$dev
+ sysfsf=$sysfsd/ipsec
+
+ # setup netdevsim since dummydev doesn't have offload support
+ modprobe netdevsim
+ check_err $?
+ if [ $ret -ne 0 ]; then
+ echo "FAIL: ipsec_offload can't load netdevsim"
+ return 1
+ fi
+
+ ip link add $dev type netdevsim
+ ip addr add $srcip dev $dev
+ ip link set $dev up
+ if [ ! -d $sysfsd ] ; then
+ echo "FAIL: ipsec_offload can't create device $dev"
+ return 1
+ fi
+ if [ ! -f $sysfsf ] ; then
+ echo "FAIL: ipsec_offload netdevsim doesn't support IPsec offload"
+ return 1
+ fi
+
+ # flush to be sure there's nothing configured
+ ip x s flush ; ip x p flush
+
+ # create offloaded SAs, both in and out
+ ip x p add dir out src $srcip/24 dst $dstip/24 \
+ tmpl proto esp src $srcip dst $dstip spi 9 \
+ mode transport reqid 42
+ check_err $?
+ ip x p add dir out src $dstip/24 dst $srcip/24 \
+ tmpl proto esp src $dstip dst $srcip spi 9 \
+ mode transport reqid 42
+ check_err $?
+
+ ip x s add proto esp src $srcip dst $dstip spi 9 \
+ mode transport reqid 42 $algo sel src $srcip/24 dst $dstip/24 \
+ offload dev $dev dir out
+ check_err $?
+ ip x s add proto esp src $dstip dst $srcip spi 9 \
+ mode transport reqid 42 $algo sel src $dstip/24 dst $srcip/24 \
+ offload dev $dev dir in
+ check_err $?
+ if [ $ret -ne 0 ]; then
+ echo "FAIL: ipsec_offload can't create SA"
+ return 1
+ fi
+
+ # does offload show up in ip output
+ lines=`ip x s list | grep -c "crypto offload parameters: dev $dev dir"`
+ if [ $lines -ne 2 ] ; then
+ echo "FAIL: ipsec_offload SA offload missing from list output"
+ check_err 1
+ fi
+
+ # use ping to exercise the Tx path
+ ping -I $dev -c 3 -W 1 -i 0 $dstip >/dev/null
+
+ # does driver have correct offload info
+ diff $sysfsf - << EOF
+SA count=2 tx=3
+sa[0] tx ipaddr=0x00000000 00000000 00000000 00000000
+sa[0] spi=0x00000009 proto=0x32 salt=0x61626364 crypt=1
+sa[0] key=0x34333231 38373635 32313039 36353433
+sa[1] rx ipaddr=0x00000000 00000000 00000000 037ba8c0
+sa[1] spi=0x00000009 proto=0x32 salt=0x61626364 crypt=1
+sa[1] key=0x34333231 38373635 32313039 36353433
+EOF
+ if [ $? -ne 0 ] ; then
+ echo "FAIL: ipsec_offload incorrect driver data"
+ check_err 1
+ fi
+
+ # does offload get removed from driver
+ ip x s flush
+ ip x p flush
+ lines=`grep -c "SA count=0" $sysfsf`
+ if [ $lines -ne 1 ] ; then
+ echo "FAIL: ipsec_offload SA not removed from driver"
+ check_err 1
+ fi
+
+ # clean up any leftovers
+ ip link del $dev
+ rmmod netdevsim
+
+ if [ $ret -ne 0 ]; then
+ echo "FAIL: ipsec_offload"
+ return 1
+ fi
+ echo "PASS: ipsec_offload"
+}
+
kci_test_gretap()
{
testns="testns"
@@ -862,6 +975,7 @@ kci_test_rtnl()
kci_test_encap
kci_test_macsec
kci_test_ipsec
+ kci_test_ipsec_offload
kci_del_dummy
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/4] netdevsim: add ipsec offload testing
From: Shannon Nelson @ 2018-06-23 0:31 UTC (permalink / raw)
To: davem, netdev, jakub.kicinski; +Cc: anders.roxell, linux-kselftest
In-Reply-To: <1529713898-9200-1-git-send-email-shannon.nelson@oracle.com>
Implement the IPsec/XFRM offload API for testing.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/netdevsim/Makefile | 4 +
drivers/net/netdevsim/ipsec.c | 345 ++++++++++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdev.c | 7 +
drivers/net/netdevsim/netdevsim.h | 37 ++++
4 files changed, 393 insertions(+)
create mode 100644 drivers/net/netdevsim/ipsec.c
diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index 449b2a1..0fee1d0 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -13,3 +13,7 @@ endif
ifneq ($(CONFIG_NET_DEVLINK),)
netdevsim-objs += devlink.o fib.o
endif
+
+ifneq ($(CONFIG_XFRM_OFFLOAD),)
+netdevsim-objs += ipsec.o
+endif
diff --git a/drivers/net/netdevsim/ipsec.c b/drivers/net/netdevsim/ipsec.c
new file mode 100644
index 0000000..ad64266
--- /dev/null
+++ b/drivers/net/netdevsim/ipsec.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Oracle and/or its affiliates. All rights reserved. */
+
+#include <net/xfrm.h>
+#include <crypto/aead.h>
+#include <linux/debugfs.h>
+#include "netdevsim.h"
+
+#define NSIM_IPSEC_AUTH_BITS 128
+
+/**
+ * nsim_ipsec_dbg_read - read for ipsec data
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t nsim_dbg_netdev_ops_read(struct file *filp,
+ char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct netdevsim *ns = filp->private_data;
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+ size_t bufsize;
+ char *buf, *p;
+ int len;
+ int i;
+
+ /* don't allow partial reads */
+ if (*ppos != 0)
+ return 0;
+
+ /* the buffer needed is
+ * (num SAs * 3 lines each * ~60 bytes per line) + one more line
+ */
+ bufsize = (ipsec->count * 4 * 60) + 60;
+ buf = kzalloc(bufsize, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ p = buf;
+ p += snprintf(p, bufsize - (p - buf),
+ "SA count=%u tx=%u\n",
+ ipsec->count, ipsec->tx);
+
+ for (i = 0; i < NSIM_IPSEC_MAX_SA_COUNT; i++) {
+ struct nsim_sa *sap = &ipsec->sa[i];
+
+ if (!sap->used)
+ continue;
+
+ p += snprintf(p, bufsize - (p - buf),
+ "sa[%i] %cx ipaddr=0x%08x %08x %08x %08x\n",
+ i, (sap->rx ? 'r' : 't'), sap->ipaddr[0],
+ sap->ipaddr[1], sap->ipaddr[2], sap->ipaddr[3]);
+ p += snprintf(p, bufsize - (p - buf),
+ "sa[%i] spi=0x%08x proto=0x%x salt=0x%08x crypt=%d\n",
+ i, be32_to_cpu(sap->xs->id.spi),
+ sap->xs->id.proto, sap->salt, sap->crypt);
+ p += snprintf(p, bufsize - (p - buf),
+ "sa[%i] key=0x%08x %08x %08x %08x\n",
+ i, sap->key[0], sap->key[1],
+ sap->key[2], sap->key[3]);
+ }
+
+ len = simple_read_from_buffer(buffer, count, ppos, buf, p - buf);
+
+ kfree(buf);
+ return len;
+}
+
+static const struct file_operations ipsec_dbg_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .read = nsim_dbg_netdev_ops_read,
+};
+
+/**
+ * nsim_ipsec_find_empty_idx - find the first unused security parameter index
+ * @ipsec: pointer to ipsec struct
+ **/
+static int nsim_ipsec_find_empty_idx(struct nsim_ipsec *ipsec)
+{
+ u32 i;
+
+ if (ipsec->count == NSIM_IPSEC_MAX_SA_COUNT)
+ return -ENOSPC;
+
+ /* search sa table */
+ for (i = 0; i < NSIM_IPSEC_MAX_SA_COUNT; i++) {
+ if (!ipsec->sa[i].used)
+ return i;
+ }
+
+ return -ENOSPC;
+}
+
+/**
+ * nsim_ipsec_parse_proto_keys - find the key and salt based on the protocol
+ * @xs: pointer to xfrm_state struct
+ * @mykey: pointer to key array to populate
+ * @mysalt: pointer to salt value to populate
+ *
+ * This copies the protocol keys and salt to our own data tables. The
+ * 82599 family only supports the one algorithm.
+ **/
+static int nsim_ipsec_parse_proto_keys(struct xfrm_state *xs,
+ u32 *mykey, u32 *mysalt)
+{
+ struct net_device *dev = xs->xso.dev;
+ unsigned char *key_data;
+ char *alg_name = NULL;
+ const char aes_gcm_name[] = "rfc4106(gcm(aes))";
+ int key_len;
+
+ if (!xs->aead) {
+ netdev_err(dev, "Unsupported IPsec algorithm\n");
+ return -EINVAL;
+ }
+
+ if (xs->aead->alg_icv_len != NSIM_IPSEC_AUTH_BITS) {
+ netdev_err(dev, "IPsec offload requires %d bit authentication\n",
+ NSIM_IPSEC_AUTH_BITS);
+ return -EINVAL;
+ }
+
+ key_data = &xs->aead->alg_key[0];
+ key_len = xs->aead->alg_key_len;
+ alg_name = xs->aead->alg_name;
+
+ if (strcmp(alg_name, aes_gcm_name)) {
+ netdev_err(dev, "Unsupported IPsec algorithm - please use %s\n",
+ aes_gcm_name);
+ return -EINVAL;
+ }
+
+ /* The key bytes come down in a bigendian array of bytes, so
+ * we don't need to do any byteswapping.
+ * 160 accounts for 16 byte key and 4 byte salt
+ */
+ if (key_len > 128) {
+ *mysalt = ((u32 *)key_data)[4];
+ } else if (key_len == 128) {
+ *mysalt = 0;
+ } else {
+ netdev_err(dev, "IPsec hw offload only supports 128 bit keys with optional 32 bit salt\n");
+ return -EINVAL;
+ }
+ memcpy(mykey, key_data, 16);
+
+ return 0;
+}
+
+/**
+ * nsim_ipsec_add_sa - program device with a security association
+ * @xs: pointer to transformer state struct
+ **/
+static int nsim_ipsec_add_sa(struct xfrm_state *xs)
+{
+ struct net_device *dev = xs->xso.dev;
+ struct netdevsim *ns = netdev_priv(dev);
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+ struct nsim_sa sa;
+ u16 sa_idx;
+ int ret;
+
+ if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
+ netdev_err(dev, "Unsupported protocol 0x%04x for ipsec offload\n",
+ xs->id.proto);
+ return -EINVAL;
+ }
+
+ if (xs->calg) {
+ netdev_err(dev, "Compression offload not supported\n");
+ return -EINVAL;
+ }
+
+ /* find the first unused index */
+ ret = nsim_ipsec_find_empty_idx(ipsec);
+ if (ret < 0) {
+ netdev_err(dev, "No space for SA in Rx table!\n");
+ return ret;
+ }
+ sa_idx = (u16)ret;
+
+ memset(&sa, 0, sizeof(sa));
+ sa.used = true;
+ sa.xs = xs;
+
+ if (sa.xs->id.proto & IPPROTO_ESP)
+ sa.crypt = xs->ealg || xs->aead;
+
+ /* get the key and salt */
+ ret = nsim_ipsec_parse_proto_keys(xs, sa.key, &sa.salt);
+ if (ret) {
+ netdev_err(dev, "Failed to get key data for SA table\n");
+ return ret;
+ }
+
+ if (xs->xso.flags & XFRM_OFFLOAD_INBOUND) {
+ sa.rx = true;
+
+ if (xs->props.family == AF_INET6)
+ memcpy(sa.ipaddr, &xs->id.daddr.a6, 16);
+ else
+ memcpy(&sa.ipaddr[3], &xs->id.daddr.a4, 4);
+ }
+
+ /* the preparations worked, so save the info */
+ memcpy(&ipsec->sa[sa_idx], &sa, sizeof(sa));
+
+ /* the XFRM stack doesn't like offload_handle == 0,
+ * so add a bitflag in case our array index is 0
+ */
+ xs->xso.offload_handle = sa_idx | NSIM_IPSEC_VALID;
+ ipsec->count++;
+
+ return 0;
+}
+
+/**
+ * nsim_ipsec_del_sa - clear out this specific SA
+ * @xs: pointer to transformer state struct
+ **/
+static void nsim_ipsec_del_sa(struct xfrm_state *xs)
+{
+ struct net_device *dev = xs->xso.dev;
+ struct netdevsim *ns = netdev_priv(dev);
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+ u16 sa_idx;
+
+ sa_idx = xs->xso.offload_handle & ~NSIM_IPSEC_VALID;
+ if (!ipsec->sa[sa_idx].used) {
+ netdev_err(dev, "Invalid SA for delete sa_idx=%d\n", sa_idx);
+ return;
+ }
+
+ memset(&ipsec->sa[sa_idx], 0, sizeof(struct nsim_sa));
+ ipsec->count--;
+}
+
+/**
+ * nsim_ipsec_offload_ok - can this packet use the xfrm hw offload
+ * @skb: current data packet
+ * @xs: pointer to transformer state struct
+ **/
+static bool nsim_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
+{
+ struct net_device *dev = xs->xso.dev;
+ struct netdevsim *ns = netdev_priv(dev);
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+
+ ipsec->ok++;
+
+ return true;
+}
+
+static const struct xfrmdev_ops nsim_xfrmdev_ops = {
+ .xdo_dev_state_add = nsim_ipsec_add_sa,
+ .xdo_dev_state_delete = nsim_ipsec_del_sa,
+ .xdo_dev_offload_ok = nsim_ipsec_offload_ok,
+};
+
+/**
+ * nsim_ipsec_tx - check Tx packet for ipsec offload
+ * @ns: pointer to ns structure
+ * @skb: current data packet
+ **/
+int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
+{
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+ struct xfrm_state *xs;
+ struct nsim_sa *tsa;
+ u32 sa_idx;
+
+ /* do we even need to check this packet? */
+ if (!skb->sp)
+ return 1;
+
+ if (unlikely(!skb->sp->len)) {
+ netdev_err(ns->netdev, "%s: no xfrm state len = %d\n",
+ __func__, skb->sp->len);
+ return 0;
+ }
+
+ xs = xfrm_input_state(skb);
+ if (unlikely(!xs)) {
+ netdev_err(ns->netdev, "%s: no xfrm_input_state() xs = %p\n",
+ __func__, xs);
+ return 0;
+ }
+
+ sa_idx = xs->xso.offload_handle & ~NSIM_IPSEC_VALID;
+ if (unlikely(sa_idx > NSIM_IPSEC_MAX_SA_COUNT)) {
+ netdev_err(ns->netdev, "%s: bad sa_idx=%d max=%d\n",
+ __func__, sa_idx, NSIM_IPSEC_MAX_SA_COUNT);
+ return 0;
+ }
+
+ tsa = &ipsec->sa[sa_idx];
+ if (unlikely(!tsa->used)) {
+ netdev_err(ns->netdev, "%s: unused sa_idx=%d\n",
+ __func__, sa_idx);
+ return 0;
+ }
+
+ if (xs->id.proto != IPPROTO_ESP && xs->id.proto != IPPROTO_AH) {
+ netdev_err(ns->netdev, "%s: unexpected proto=%d\n",
+ __func__, xs->id.proto);
+ return 0;
+ }
+
+ ipsec->tx++;
+
+ return 1;
+}
+
+/**
+ * nsim_ipsec_init - initialize security registers for IPSec operation
+ * @ns: board private structure
+ **/
+void nsim_ipsec_init(struct netdevsim *ns)
+{
+ ns->netdev->xfrmdev_ops = &nsim_xfrmdev_ops;
+
+#define NSIM_ESP_FEATURES (NETIF_F_HW_ESP | \
+ NETIF_F_HW_ESP_TX_CSUM | \
+ NETIF_F_GSO_ESP)
+
+ ns->netdev->features |= NSIM_ESP_FEATURES;
+ ns->netdev->hw_enc_features |= NSIM_ESP_FEATURES;
+
+ ns->ipsec.pfile = debugfs_create_file("ipsec", 0400, ns->ddir, ns,
+ &ipsec_dbg_fops);
+}
+
+void nsim_ipsec_teardown(struct netdevsim *ns)
+{
+ struct nsim_ipsec *ipsec = &ns->ipsec;
+
+ if (ipsec->count)
+ netdev_err(ns->netdev, "%s: tearing down IPsec offload with %d SAs left\n",
+ __func__, ipsec->count);
+ debugfs_remove_recursive(ipsec->pfile);
+}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index ec68f38..6ce8604d 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -171,6 +171,8 @@ static int nsim_init(struct net_device *dev)
if (err)
goto err_unreg_dev;
+ nsim_ipsec_init(ns);
+
return 0;
err_unreg_dev:
@@ -186,6 +188,7 @@ static void nsim_uninit(struct net_device *dev)
{
struct netdevsim *ns = netdev_priv(dev);
+ nsim_ipsec_teardown(ns);
nsim_devlink_teardown(ns);
debugfs_remove_recursive(ns->ddir);
nsim_bpf_uninit(ns);
@@ -203,11 +206,15 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct netdevsim *ns = netdev_priv(dev);
+ if (!nsim_ipsec_tx(ns, skb))
+ goto out;
+
u64_stats_update_begin(&ns->syncp);
ns->tx_packets++;
ns->tx_bytes += skb->len;
u64_stats_update_end(&ns->syncp);
+out:
dev_kfree_skb(skb);
return NETDEV_TX_OK;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 3a8581a..1708dee 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -29,6 +29,29 @@ struct bpf_prog;
struct dentry;
struct nsim_vf_config;
+#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
+#define NSIM_IPSEC_MAX_SA_COUNT 33
+#define NSIM_IPSEC_VALID BIT(31)
+
+struct nsim_sa {
+ struct xfrm_state *xs;
+ __be32 ipaddr[4];
+ u32 key[4];
+ u32 salt;
+ bool used;
+ bool crypt;
+ bool rx;
+};
+
+struct nsim_ipsec {
+ struct nsim_sa sa[NSIM_IPSEC_MAX_SA_COUNT];
+ struct dentry *pfile;
+ u32 count;
+ u32 tx;
+ u32 ok;
+};
+#endif
+
struct netdevsim {
struct net_device *netdev;
@@ -67,6 +90,9 @@ struct netdevsim {
#if IS_ENABLED(CONFIG_NET_DEVLINK)
struct devlink *devlink;
#endif
+#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
+ struct nsim_ipsec ipsec;
+#endif
};
extern struct dentry *nsim_ddir;
@@ -147,6 +173,17 @@ static inline void nsim_devlink_exit(void)
}
#endif
+#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
+void nsim_ipsec_init(struct netdevsim *ns);
+void nsim_ipsec_teardown(struct netdevsim *ns);
+int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb);
+#else
+static inline void nsim_ipsec_init(struct netdevsim *ns) {};
+static inline void nsim_ipsec_teardown(struct netdevsim *ns) {};
+static inline int nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
+ { return 1; };
+#endif
+
static inline struct netdevsim *to_nsim(struct device *ptr)
{
return container_of(ptr, struct netdevsim, dev);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/4] Updates for ipsec selftests
From: Shannon Nelson @ 2018-06-23 0:31 UTC (permalink / raw)
To: davem, netdev, jakub.kicinski; +Cc: anders.roxell, linux-kselftest
Fix up the existing ipsec selftest and add tests for
the ipsec offload driver API.
Shannon Nelson (4):
selftests: rtnetlink: clear the return code at start of ipsec test
selftests: rtnetlink: use dummydev as a test device
netdevsim: add ipsec offload testing
selftests: rtnetlink: add ipsec offload API test
drivers/net/netdevsim/Makefile | 4 +
drivers/net/netdevsim/ipsec.c | 345 +++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdev.c | 7 +
drivers/net/netdevsim/netdevsim.h | 37 ++++
tools/testing/selftests/net/rtnetlink.sh | 132 +++++++++++-
5 files changed, 518 insertions(+), 7 deletions(-)
create mode 100644 drivers/net/netdevsim/ipsec.c
--
2.7.4
^ permalink raw reply
* [PATCH net-next 2/4] selftests: rtnetlink: use dummydev as a test device
From: Shannon Nelson @ 2018-06-23 0:31 UTC (permalink / raw)
To: davem, netdev, jakub.kicinski; +Cc: anders.roxell, linux-kselftest
In-Reply-To: <1529713898-9200-1-git-send-email-shannon.nelson@oracle.com>
We really shouldn't mess with local system settings, so let's
use the already created dummy device instead for ipsec testing.
Oh, and let's put the temp file into a proper directory.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
tools/testing/selftests/net/rtnetlink.sh | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 261a981..15948cf 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -523,21 +523,19 @@ kci_test_macsec()
kci_test_ipsec()
{
ret=0
-
- # find an ip address on this machine and make up a destination
- srcip=`ip -o addr | awk '/inet / { print $4; }' | grep -v "^127" | head -1 | cut -f1 -d/`
- net=`echo $srcip | cut -f1-3 -d.`
- base=`echo $srcip | cut -f4 -d.`
- dstip="$net."`expr $base + 1`
-
algo="aead rfc4106(gcm(aes)) 0x3132333435363738393031323334353664636261 128"
+ srcip=192.168.123.1
+ dstip=192.168.123.2
+ spi=7
+
+ ip addr add $srcip dev $devdummy
# flush to be sure there's nothing configured
ip x s flush ; ip x p flush
check_err $?
# start the monitor in the background
- tmpfile=`mktemp ipsectestXXX`
+ tmpfile=`mktemp /var/run/ipsectestXXX`
mpid=`(ip x m > $tmpfile & echo $!) 2>/dev/null`
sleep 0.2
@@ -601,6 +599,7 @@ kci_test_ipsec()
check_err $?
ip x p flush
check_err $?
+ ip addr del $srcip/32 dev $devdummy
if [ $ret -ne 0 ]; then
echo "FAIL: ipsec"
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/4] selftests: rtnetlink: clear the return code at start of ipsec test
From: Shannon Nelson @ 2018-06-23 0:31 UTC (permalink / raw)
To: davem, netdev, jakub.kicinski; +Cc: anders.roxell, linux-kselftest
In-Reply-To: <1529713898-9200-1-git-send-email-shannon.nelson@oracle.com>
Following the custom from the other functions, clear the global
ret code before starting the test so as to not have previously
failed tests cause us to think this test has failed.
Reported-by: Anders Roxell <anders.roxell@linaro.org>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
tools/testing/selftests/net/rtnetlink.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index b33a371..261a981 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -522,6 +522,8 @@ kci_test_macsec()
#-------------------------------------------------------------------
kci_test_ipsec()
{
+ ret=0
+
# find an ip address on this machine and make up a destination
srcip=`ip -o addr | awk '/inet / { print $4; }' | grep -v "^127" | head -1 | cut -f1 -d/`
net=`echo $srcip | cut -f1-3 -d.`
--
2.7.4
^ permalink raw reply related
* Re: [PATCH bpf-next 2/3] bpf: btf: add btf json print functionality
From: Martin KaFai Lau @ 2018-06-23 0:26 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Okash Khawaja, Daniel Borkmann, Alexei Starovoitov, Yonghong Song,
Quentin Monnet, David S. Miller, netdev, kernel-team,
linux-kernel
In-Reply-To: <20180622163200.20564ec4@cakuba.netronome.com>
On Fri, Jun 22, 2018 at 04:32:00PM -0700, Jakub Kicinski wrote:
> On Fri, 22 Jun 2018 15:54:08 -0700, Martin KaFai Lau wrote:
> > > > > > > > > > > "value": ["0x02","0x00","0x00","0x00","0x00","0x00","0x00","0x00"
> > > > > > > > > > > ],
> > > > > > > > > > > "value_struct":{
> > > > > > > > > > > "src_ip":2,
> > > > > > If for the same map the user changes the "src_ip" to an array of int[4]
> > > > > > later (e.g. to support ipv6), it will become "src_ip": [1, 2, 3, 4].
> > > > > > Is it breaking backward compat?
> > > > > > i.e.
> > > > > > struct five_tuples {
> > > > > > - int src_ip;
> > > > > > + int src_ip[4];
> > > > > > /* ... */
> > > > > > };
> > > > >
> > > > > Well, it is breaking backward compat, but it's the program doing it,
> > > > > not bpftool :) BTF changes so does the output.
> > > > As we see, the key/value's btf-output is inherently not backward compat.
> > > > Hence, "-j" and "-p" will stay as is. The whole existing json will
> > > > be backward compat instead of only partly backward compat.
> > >
> > > No. There is a difference between user of a facility changing their
> > > input and kernel/libraries providing different output in response to
> > > that, and the libraries suddenly changing the output on their own.
> > >
> > > Your example is like saying if user started using IPv6 addresses
> > > instead of IPv4 the netlink attributes in dumps will be different so
> > > kernel didn't keep backwards compat. While what you're doing is more
> > > equivalent to dropping support for old ioctl interfaces because there
> > > is a better mechanism now.
> > Sorry, I don't follow this. I don't see netlink suffer json issue like
> > the one on "key" and "value".
> >
> > All I can grasp is, the json should normally be backward compat but now
> > we are saying anything added by btf-output is an exception because
> > the script parsing it will treat it differently than "key" and "value"
>
> Backward compatibility means that if I run *the same* program against
> different kernels/libraries it continues to work. If someone decides
> to upgrade their program to work with IPv6 (which was your example)
> obviously there is no way system as a whole will look 1:1 the same.
>
> > > BTF in JSON is very useful, and will help people who writes simple
> > > orchestration/scripts based on bpftool *a* *lot*. I really appreciate
> > Can you share what the script will do? I want to understand why
> > it cannot directly use the BTF format and the map data.
>
> Think about a python script which wants to read a counter in a map.
> Right now it would have to get the BTF, find out which bytes are the
> counter, then convert the bytes into a larger int. With JSON BTF if
> just does entry["formatted"]["value"]["counter"].
>
> Real life example from my test code (conversion of 3 element counter
> array):
>
> def str2int(strtab):
> inttab = []
> for i in strtab:
> inttab.append(int(i, 16))
> ba = bytearray(inttab)
> if len(strtab) == 4:
> fmt = "I"
> elif len(strtab) == 8:
> fmt = "Q"
> else:
> raise Exception("String array of len %d can't be unpacked to an int" %
> (len(strtab)))
> return struct.unpack(fmt, ba)[0]
>
> def convert(elems, idx):
> val = []
> for i in range(3):
> part = elems[idx]["value"][i * length:(i + 1) * length]
> val.append(str2int(part))
> return val
>
> With BTF it would be:
>
> elems[idx]["formatted"]["value"]
>
> Which is fairly awesome.
Thanks for the example. Agree that with BTF, things are easier in general.
btw, what more awesome is,
#> bpftool map find id 100 key 1
{
"counter_x": 1,
"counter_y": 10
}
>
> > > this addition to bpftool and will start using it myself as soon as it
> > > lands. I'm not sure why the reluctance to slightly change the output
> > > format?
> > The initial change argument is because the json has to be backward compat.
> >
> > Then we show that btf-output is inherently not backward compat, so
> > printing it in json does not make sense at all.
> >
> > However, now it is saying part of it does not have to be backward compat.
>
> Compatibility of "formatted" member is defined as -> fields broken out
> according to BTF. So it is backward compatible. The definition of
> "value" member is -> an array of unfortunately formatted array of
> ugly hex strings :(
>
> > I am fine putting it under "formatted" for "-j" or "-p" if that is the
> > case, other than the double output is still confusing. Lets wait for
> > Okash's input.
> >
> > At the same time, the same output will be used as the default plaintext
> > output when BTF is available. Then the plaintext BTF output
> > will not be limited by the json restrictions when we want
> > to improve human readability later. Apparently, the
> > improvements on plaintext will not be always applicable
> > to json output.
>
^ permalink raw reply
* Re: [virtio-dev] Re: [Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
From: Siwei Liu @ 2018-06-23 0:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, konrad.wilk,
Jakub Kicinski, Samudrala, Sridhar, Cornelia Huck, qemu-devel,
virtualization, Venu Busireddy, Netdev, boris.ostrovsky,
aaron.f.brown, Joao Martins
In-Reply-To: <CADGSJ211pWKYnuuzcgUqJZiHU+=7H3oQSpp1TnE=+EBjkdmCSQ@mail.gmail.com>
On Fri, Jun 22, 2018 at 4:40 PM, Siwei Liu <loseweigh@gmail.com> wrote:
> On Fri, Jun 22, 2018 at 3:25 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Fri, Jun 22, 2018 at 02:51:11PM -0700, Siwei Liu wrote:
>>> On Fri, Jun 22, 2018 at 2:29 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> > On Fri, Jun 22, 2018 at 01:03:04PM -0700, Siwei Liu wrote:
>>> >> On Fri, Jun 22, 2018 at 1:00 PM, Siwei Liu <loseweigh@gmail.com> wrote:
>>> >> > On Thu, Jun 21, 2018 at 7:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> >> >> On Thu, Jun 21, 2018 at 06:21:55PM -0700, Siwei Liu wrote:
>>> >> >>> On Thu, Jun 21, 2018 at 7:59 AM, Cornelia Huck <cohuck@redhat.com> wrote:
>>> >> >>> > On Wed, 20 Jun 2018 22:48:58 +0300
>>> >> >>> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>> >> >>> >
>>> >> >>> >> On Wed, Jun 20, 2018 at 06:06:19PM +0200, Cornelia Huck wrote:
>>> >> >>> >> > In any case, I'm not sure anymore why we'd want the extra uuid.
>>> >> >>> >>
>>> >> >>> >> It's mostly so we can have e.g. multiple devices with same MAC
>>> >> >>> >> (which some people seem to want in order to then use
>>> >> >>> >> then with different containers).
>>> >> >>> >>
>>> >> >>> >> But it is also handy for when you assign a PF, since then you
>>> >> >>> >> can't set the MAC.
>>> >> >>> >>
>>> >> >>> >
>>> >> >>> > OK, so what about the following:
>>> >> >>> >
>>> >> >>> > - introduce a new feature bit, VIRTIO_NET_F_STANDBY_UUID that indicates
>>> >> >>> > that we have a new uuid field in the virtio-net config space
>>> >> >>> > - in QEMU, add a property for virtio-net that allows to specify a uuid,
>>> >> >>> > offer VIRTIO_NET_F_STANDBY_UUID if set
>>> >> >>> > - when configuring, set the property to the group UUID of the vfio-pci
>>> >> >>> > device
>>> >> >>>
>>> >> >>> If feature negotiation fails on VIRTIO_NET_F_STANDBY_UUID, is it safe
>>> >> >>> to still expose UUID in the config space on virtio-pci?
>>> >> >>
>>> >> >>
>>> >> >> Yes but guest is not supposed to read it.
>>> >> >>
>>> >> >>> I'm not even sure if it's sane to expose group UUID on the PCI bridge
>>> >> >>> where the corresponding vfio-pci device attached to for a guest which
>>> >> >>> doesn't support the feature (legacy).
>>> >> >>>
>>> >> >>> -Siwei
>>> >> >>
>>> >> >> Yes but you won't add the primary behind such a bridge.
>>> >> >
>>> >> > I assume the UUID feature is a new one besides the exiting
>>> >> > VIRTIO_NET_F_STANDBY feature, where I think the current proposal is,
>>> >> > if UUID feature is present and supported by guest, we'll do pairing
>>> >> > based on UUID; if UUID feature present
>>> >> ^^^^^^^ is NOT present
>>> >
>>> > Let's go back for a bit.
>>> >
>>> > What is wrong with matching by mac?
>>> >
>>> > 1. Does not allow multiple NICs with same mac
>>> > 2. Requires MAC to be programmed by host in the PT device
>>> > (which is often possible with VFs but isn't possible with all PT
>>> > devices)
>>>
>>> Might not neccessarily be something wrong, but it's very limited to
>>> prohibit the MAC of VF from changing when enslaved by failover.
>>
>> You mean guest changing MAC? I'm not sure why we prohibit that.
>
> I think Sridhar and Jiri might be better person to answer it. My
> impression was that sync'ing the MAC address change between all 3
> devices is challenging, as the failover driver uses MAC address to
> match net_device internally.
>
>>
>>
>>> The
>>> same as you indicated on the PT device. I suspect the same is
>>> prohibited on even virtio-net and failover is because of the same
>>> limitation.
>>>
>>> >
>>> > Both issues are up to host: if host needs them it needs the UUID
>>> > feature, if not MAC matching is sufficient.
>>>
>>> I know. What I'm saying is that we rely on STANDBY feature to control
>>> the visibility of the primary, not the UUID feature.
>>>
>>> -Siwei
>>
>> And what I'm saying is that it's up to a host policy.
>
> So lets say host policy explicitly requires UUID but the guest does
> not support it. The guest could be a legacy guest with no STANDBY
> support, or a relevately recent guest with STANDBY support but without
> the UUID support. In either case, since host asked for UUID matching
> explicitly, maybe because it requires different NIC using same MAC, so
> it has to override the negotiation result of STANDBY, even if STANDBY
> is set and supported? That will end up with inter-feature dependency
> over STANDBY, as you see.
I forgot to mention the above has the assumption that we expose both
STANDBY and UUID feature to QEMU user. In fact, as we're going towards
not exposing the STANDBY feature directly to user, UUID may be always
required to enable STANDBY. If not, how do we make sure QEMU can
control the visibility of primary device? Something to be confirmed
before implementing the code.
-Siwei
>
> -Siwei
>
>>
>>> >
>>> >
>>> >> > or not supported by guest,
>>> >> > we'll still plug in the VF (if guest supports the _F_STANDBY feature)
>>> >> > but the pairing will be fallback to using MAC address. Are you saying
>>> >> > you don't even want to plug in the primary when the UUID feature is
>>> >> > not supported by guest? Does the feature negotiation UUID have to
>>> >> > depend on STANDBY being set, or the other way around? I thought that
>>> >> > just the absence of STANDBY will prevent primary to get exposed to the
>>> >> > guest.
>>> >> >
>>> >> > -Siwei
>>> >> >
>>> >> >>
>>> >> >>>
>>> >> >>> > - in the guest, use the uuid from the virtio-net device's config space
>>> >> >>> > if applicable; else, fall back to matching by MAC as done today
>>> >
^ 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