* [PATCH net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAGS_UNSTABLE_PHC
@ 2021-12-08 4:42 Hangbin Liu
2021-12-08 4:42 ` [PATCH net-next 1/2] net_tstamp: add new " Hangbin Liu
2021-12-08 4:42 ` [PATCH net-next 2/2] Bonding: force user to add HWTSTAMP_FLAGS_UNSTABLE_PHC when get/set HWTSTAMP Hangbin Liu
0 siblings, 2 replies; 10+ messages in thread
From: Hangbin Liu @ 2021-12-08 4:42 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S . Miller,
Jakub Kicinski, Richard Cochran, Heiner Kallweit, Hangbin Liu
This patchset add a new hwtstamp_config flag HWTSTAMP_FLAGS_UNSTABLE_PHC.
When user want to get unstable PHC index, like bond's active interface PHC,
they need to add this flag and aware the PHC index may changed.
Hangbin Liu (2):
net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
Bonding: force user to add HWTSTAMP_FLAGS_UNSTABLE_PHC when get/set
HWTSTAMP
drivers/net/bonding/bond_main.c | 33 ++++++++++++-------
.../net/dsa/hirschmann/hellcreek_hwtstamp.c | 4 ---
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 4 ---
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 3 --
.../net/ethernet/aquantia/atlantic/aq_main.c | 3 --
.../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 5 ---
drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 3 --
drivers/net/ethernet/broadcom/tg3.c | 3 --
drivers/net/ethernet/cadence/macb_ptp.c | 4 ---
.../net/ethernet/cavium/liquidio/lio_main.c | 3 --
.../ethernet/cavium/liquidio/lio_vf_main.c | 3 --
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 3 --
.../net/ethernet/cavium/thunder/nicvf_main.c | 4 ---
drivers/net/ethernet/engleder/tsnep_ptp.c | 3 --
drivers/net/ethernet/freescale/fec_ptp.c | 4 ---
drivers/net/ethernet/freescale/gianfar.c | 4 ---
drivers/net/ethernet/intel/e1000e/netdev.c | 4 ---
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 4 ---
drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ---
drivers/net/ethernet/intel/igb/igb_ptp.c | 4 ---
drivers/net/ethernet/intel/igc/igc_ptp.c | 4 ---
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 4 ---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 --
.../ethernet/marvell/octeontx2/nic/otx2_pf.c | 4 ---
.../net/ethernet/mellanox/mlx4/en_netdev.c | 4 ---
drivers/net/ethernet/microchip/lan743x_ptp.c | 6 ----
drivers/net/ethernet/mscc/ocelot.c | 4 ---
.../net/ethernet/neterion/vxge/vxge-main.c | 4 ---
.../ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 3 --
drivers/net/ethernet/qlogic/qede/qede_ptp.c | 5 ---
drivers/net/ethernet/renesas/ravb_main.c | 4 ---
drivers/net/ethernet/sfc/ptp.c | 3 --
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ---
drivers/net/ethernet/ti/cpsw_priv.c | 4 ---
drivers/net/ethernet/ti/netcp_ethss.c | 4 ---
drivers/net/ethernet/xscale/ixp4xx_eth.c | 3 --
drivers/net/phy/dp83640.c | 3 --
drivers/net/phy/mscc/mscc_ptp.c | 3 --
drivers/ptp/ptp_ines.c | 4 ---
include/uapi/linux/net_tstamp.h | 15 ++++++++-
net/core/dev_ioctl.c | 3 --
41 files changed, 35 insertions(+), 158 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 4:42 [PATCH net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAGS_UNSTABLE_PHC Hangbin Liu
@ 2021-12-08 4:42 ` Hangbin Liu
2021-12-08 6:08 ` Jakub Kicinski
2021-12-08 15:20 ` Richard Cochran
2021-12-08 4:42 ` [PATCH net-next 2/2] Bonding: force user to add HWTSTAMP_FLAGS_UNSTABLE_PHC when get/set HWTSTAMP Hangbin Liu
1 sibling, 2 replies; 10+ messages in thread
From: Hangbin Liu @ 2021-12-08 4:42 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S . Miller,
Jakub Kicinski, Richard Cochran, Heiner Kallweit, Hangbin Liu
Since commit 94dd016ae538 ("bond: pass get_ts_info and SIOC[SG]HWTSTAMP
ioctl to active device") the user could get bond active interface's
PHC index directly. But when there is a failover, the bond active
interface will change, thus the PHC index is also changed. This may
break the user's program if they did not update the PHC timely.
This patch adds a new hwtstamp_config flag HWTSTAMP_FLAGS_UNSTABLE_PHC.
When the user wants to use get unstable PHC index, like bond active
interface's PHC, they need to add this flag and be aware the PHC index may
be changed.
With the new flag. All flag checks in current drivers need to be removed.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c | 4 ----
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 4 ----
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 3 ---
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 3 ---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 5 -----
drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 3 ---
drivers/net/ethernet/broadcom/tg3.c | 3 ---
drivers/net/ethernet/cadence/macb_ptp.c | 4 ----
drivers/net/ethernet/cavium/liquidio/lio_main.c | 3 ---
.../net/ethernet/cavium/liquidio/lio_vf_main.c | 3 ---
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 3 ---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ----
drivers/net/ethernet/engleder/tsnep_ptp.c | 3 ---
drivers/net/ethernet/freescale/fec_ptp.c | 4 ----
drivers/net/ethernet/freescale/gianfar.c | 4 ----
drivers/net/ethernet/intel/e1000e/netdev.c | 4 ----
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 4 ----
drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ----
drivers/net/ethernet/intel/igb/igb_ptp.c | 4 ----
drivers/net/ethernet/intel/igc/igc_ptp.c | 4 ----
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 4 ----
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 ---
.../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 4 ----
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 4 ----
drivers/net/ethernet/microchip/lan743x_ptp.c | 6 ------
drivers/net/ethernet/mscc/ocelot.c | 4 ----
drivers/net/ethernet/neterion/vxge/vxge-main.c | 4 ----
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 3 ---
drivers/net/ethernet/qlogic/qede/qede_ptp.c | 5 -----
drivers/net/ethernet/renesas/ravb_main.c | 4 ----
drivers/net/ethernet/sfc/ptp.c | 3 ---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ----
drivers/net/ethernet/ti/cpsw_priv.c | 4 ----
drivers/net/ethernet/ti/netcp_ethss.c | 4 ----
drivers/net/ethernet/xscale/ixp4xx_eth.c | 3 ---
drivers/net/phy/dp83640.c | 3 ---
drivers/net/phy/mscc/mscc_ptp.c | 3 ---
drivers/ptp/ptp_ines.c | 4 ----
include/uapi/linux/net_tstamp.h | 15 ++++++++++++++-
net/core/dev_ioctl.c | 3 ---
40 files changed, 14 insertions(+), 146 deletions(-)
diff --git a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
index 40b41c794dfa..b3bc948d6145 100644
--- a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
+++ b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
@@ -52,10 +52,6 @@ static int hellcreek_set_hwtstamp_config(struct hellcreek *hellcreek, int port,
*/
clear_bit_unlock(HELLCREEK_HWTSTAMP_ENABLED, &ps->state);
- /* Reserved for future extensions */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_ON:
tx_tstamp_enable = true;
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 8f74ffc7a279..389f8a6ec0ab 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -100,10 +100,6 @@ static int mv88e6xxx_set_hwtstamp_config(struct mv88e6xxx_chip *chip, int port,
*/
clear_bit_unlock(MV88E6XXX_HWTSTAMP_ENABLED, &ps->state);
- /* reserved for future extensions */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
tstamp_enable = false;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 30d24d19f40d..492ac383f16d 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -1508,9 +1508,6 @@ static int xgbe_set_hwtstamp_settings(struct xgbe_prv_data *pdata,
if (copy_from_user(&config, ifreq->ifr_data, sizeof(config)))
return -EFAULT;
- if (config.flags)
- return -EINVAL;
-
mac_tscr = 0;
switch (config.tx_type) {
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index e22935ce9573..e65ce7199dac 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -231,9 +231,6 @@ static void aq_ndev_set_multicast_settings(struct net_device *ndev)
static int aq_ndev_config_hwtstamp(struct aq_nic_s *aq_nic,
struct hwtstamp_config *config)
{
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
case HWTSTAMP_TX_ON:
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index aec666e97683..651bc1d7a57a 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -15356,11 +15356,6 @@ static int bnx2x_hwtstamp_ioctl(struct bnx2x *bp, struct ifreq *ifr)
DP(BNX2X_MSG_PTP, "Requested tx_type: %d, requested rx_filters = %d\n",
config.tx_type, config.rx_filter);
- if (config.flags) {
- BNX2X_ERR("config.flags is reserved for future use\n");
- return -EINVAL;
- }
-
bp->hwtstamp_ioctl_called = true;
bp->tx_type = config.tx_type;
bp->rx_filter = config.rx_filter;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
index 8388be119f9a..48520967746f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
@@ -417,9 +417,6 @@ int bnxt_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
if (copy_from_user(&stmpconf, ifr->ifr_data, sizeof(stmpconf)))
return -EFAULT;
- if (stmpconf.flags)
- return -EINVAL;
-
if (stmpconf.tx_type != HWTSTAMP_TX_ON &&
stmpconf.tx_type != HWTSTAMP_TX_OFF)
return -ERANGE;
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 283f3c1f1195..c28f8cc00d1c 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -13806,9 +13806,6 @@ static int tg3_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
if (copy_from_user(&stmpconf, ifr->ifr_data, sizeof(stmpconf)))
return -EFAULT;
- if (stmpconf.flags)
- return -EINVAL;
-
if (stmpconf.tx_type != HWTSTAMP_TX_ON &&
stmpconf.tx_type != HWTSTAMP_TX_OFF)
return -ERANGE;
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index 095c5a2144a7..fb6b27f46b15 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -464,10 +464,6 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
sizeof(*tstamp_config)))
return -EFAULT;
- /* reserved for future extensions */
- if (tstamp_config->flags)
- return -EINVAL;
-
switch (tstamp_config->tx_type) {
case HWTSTAMP_TX_OFF:
break;
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 12eee2bc7f5c..ba28aa444e5a 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -2114,9 +2114,6 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&conf, ifr->ifr_data, sizeof(conf)))
return -EFAULT;
- if (conf.flags)
- return -EINVAL;
-
switch (conf.tx_type) {
case HWTSTAMP_TX_ON:
case HWTSTAMP_TX_OFF:
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index c607756b731f..568f211d91cc 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -1254,9 +1254,6 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&conf, ifr->ifr_data, sizeof(conf)))
return -EFAULT;
- if (conf.flags)
- return -EINVAL;
-
switch (conf.tx_type) {
case HWTSTAMP_TX_ON:
case HWTSTAMP_TX_OFF:
diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
index 4b4ffdd1044d..103591dcea1c 100644
--- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
@@ -702,9 +702,6 @@ static int octeon_mgmt_ioctl_hwtstamp(struct net_device *netdev,
if (copy_from_user(&config, rq->ifr_data, sizeof(config)))
return -EFAULT;
- if (config.flags) /* reserved for future extensions */
- return -EINVAL;
-
/* Check the status of hardware for tiemstamps */
if (OCTEON_IS_MODEL(OCTEON_CN6XXX)) {
/* Get the current state of the PTP clock */
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index bb45d5df2856..63191692f624 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1917,10 +1917,6 @@ static int nicvf_config_hwtstamp(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
case HWTSTAMP_TX_ON:
diff --git a/drivers/net/ethernet/engleder/tsnep_ptp.c b/drivers/net/ethernet/engleder/tsnep_ptp.c
index 4bfb4d8508f5..eaad453d487e 100644
--- a/drivers/net/ethernet/engleder/tsnep_ptp.c
+++ b/drivers/net/ethernet/engleder/tsnep_ptp.c
@@ -31,9 +31,6 @@ int tsnep_ptp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
case HWTSTAMP_TX_ON:
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index d71eac7e1924..af99017a5453 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -473,10 +473,6 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
fep->hwts_tx_en = 0;
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index acab58fd3db3..206b7a35eaf5 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -2076,10 +2076,6 @@ static int gfar_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
priv->hwts_tx_en = 0;
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 44e2dc8328a2..635a95927e93 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3614,10 +3614,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP))
return -EINVAL;
- /* flags reserved for future extensions - must be zero */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
tsync_tx_ctl = 0;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 09b1d5aed1c9..61e5789d78db 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -1205,10 +1205,6 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
INIT_WORK(&pf->ptp_extts0_work, i40e_ptp_extts0_work);
- /* Reserved for future extensions. */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
pf->ptp_tx = false;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index bf7247c6f58e..dfc7c830acf6 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1205,10 +1205,6 @@ int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
static int
ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
{
- /* Reserved for future extensions. */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
ice_set_tx_tstamp(pf, false);
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 0011b15e678c..0ac4cc5eaa2d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1015,10 +1015,6 @@ static int igb_ptp_set_timestamp_mode(struct igb_adapter *adapter,
bool is_l2 = false;
u32 regval;
- /* reserved for future extensions */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
tsync_tx_ctl = 0;
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 30568e3544cd..71813fa8f928 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -560,10 +560,6 @@ static void igc_ptp_enable_tx_timestamp(struct igc_adapter *adapter)
static int igc_ptp_set_timestamp_mode(struct igc_adapter *adapter,
struct hwtstamp_config *config)
{
- /* reserved for future extensions */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
igc_ptp_disable_tx_timestamp(adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 23ddfd79fc8b..336426a67ac1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -992,10 +992,6 @@ static int ixgbe_ptp_set_timestamp_mode(struct ixgbe_adapter *adapter,
bool is_l2 = false;
u32 regval;
- /* reserved for future extensions */
- if (config->flags)
- return -EINVAL;
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
tsync_tx_ctl = 0;
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 252e215a14f1..03f4a1b1f2a4 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5142,9 +5142,6 @@ static int mvpp2_set_ts_config(struct mvpp2_port *port, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- if (config.flags)
- return -EINVAL;
-
if (config.tx_type != HWTSTAMP_TX_OFF &&
config.tx_type != HWTSTAMP_TX_ON)
return -ERANGE;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 1333edf1c361..6080ebd9bd94 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2002,10 +2002,6 @@ int otx2_config_hwtstamp(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
otx2_config_hw_tx_tstamp(pfvf, false);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index f1c10f2bda78..ad1e4caf48bf 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2427,10 +2427,6 @@ static int mlx4_en_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
/* device doesn't support time stamping */
if (!(mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_TS))
return -EINVAL;
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 9380e396f648..8b7a8d879083 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -1305,12 +1305,6 @@ int lan743x_ptp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
return -EFAULT;
- if (config.flags) {
- netif_warn(adapter, drv, adapter->netdev,
- "ignoring hwtstamp_config.flags == 0x%08X, expected 0\n",
- config.flags);
- }
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
for (index = 0; index < LAN743X_MAX_TX_CHANNELS;
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index b1856d8c944b..0be74c823d5e 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1602,10 +1602,6 @@ int ocelot_hwstamp_set(struct ocelot *ocelot, int port, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- /* reserved for future extensions */
- if (cfg.flags)
- return -EINVAL;
-
/* Tx type sanity check */
switch (cfg.tx_type) {
case HWTSTAMP_TX_ON:
diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 1969009a91e7..2c2e9e56ed4e 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -3159,10 +3159,6 @@ static int vxge_hwtstamp_set(struct vxgedev *vdev, void __user *data)
if (copy_from_user(&config, data, sizeof(config)))
return -EFAULT;
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
/* Transmit HW Timestamp not supported */
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
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 71d234291fc5..1dc40c537281 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
@@ -210,9 +210,6 @@ static int hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- if (cfg.flags) /* reserved for future extensions */
- return -EINVAL;
-
/* Get ieee1588's dev information */
pdev = adapter->ptp_pdev;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ptp.c b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
index 8c28fabb0ff6..39176e765767 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ptp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
@@ -304,11 +304,6 @@ int qede_ptp_hw_ts(struct qede_dev *edev, struct ifreq *ifr)
"HWTSTAMP IOCTL: Requested tx_type = %d, requested rx_filters = %d\n",
config.tx_type, config.rx_filter);
- if (config.flags) {
- DP_ERR(edev, "config.flags is reserved for future use\n");
- return -EINVAL;
- }
-
ptp->hw_ts_ioctl_called = 1;
ptp->tx_type = config.tx_type;
ptp->rx_filter = config.rx_filter;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ce09bd45527e..b215cde68e10 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2221,10 +2221,6 @@ static int ravb_hwtstamp_set(struct net_device *ndev, struct ifreq *req)
if (copy_from_user(&config, req->ifr_data, sizeof(config)))
return -EFAULT;
- /* Reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
tstamp_tx_ctrl = 0;
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 797e51802ccb..f0ef515e2ade 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1765,9 +1765,6 @@ static int efx_ptp_ts_init(struct efx_nic *efx, struct hwtstamp_config *init)
{
int rc;
- if (init->flags)
- return -EINVAL;
-
if ((init->tx_type != HWTSTAMP_TX_OFF) &&
(init->tx_type != HWTSTAMP_TX_ON))
return -ERANGE;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4e05c1d92935..e4d2748592ee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -636,10 +636,6 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
__func__, config.flags, config.tx_type, config.rx_filter);
- /* reserved for future extensions */
- if (config.flags)
- return -EINVAL;
-
if (config.tx_type != HWTSTAMP_TX_OFF &&
config.tx_type != HWTSTAMP_TX_ON)
return -ERANGE;
diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
index c99dd9735087..8624a044776f 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.c
+++ b/drivers/net/ethernet/ti/cpsw_priv.c
@@ -626,10 +626,6 @@ static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- /* reserved for future extensions */
- if (cfg.flags)
- return -EINVAL;
-
if (cfg.tx_type != HWTSTAMP_TX_OFF && cfg.tx_type != HWTSTAMP_TX_ON)
return -ERANGE;
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 33c1592d5381..751fb0bc65c5 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -2654,10 +2654,6 @@ static int gbe_hwtstamp_set(struct gbe_intf *gbe_intf, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- /* reserved for future extensions */
- if (cfg.flags)
- return -EINVAL;
-
switch (cfg.tx_type) {
case HWTSTAMP_TX_OFF:
gbe_dev->tx_ts_enabled = 0;
diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index 65fdad1107fc..df77a22d1b81 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -382,9 +382,6 @@ static int hwtstamp_set(struct net_device *netdev, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- if (cfg.flags) /* reserved for future extensions */
- return -EINVAL;
-
ret = ixp46x_ptp_find(&port->timesync_regs, &port->phc_index);
if (ret)
return ret;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 705c16675b80..c2d1a85ec559 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1235,9 +1235,6 @@ static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- if (cfg.flags) /* reserved for future extensions */
- return -EINVAL;
-
if (cfg.tx_type < 0 || cfg.tx_type > HWTSTAMP_TX_ONESTEP_SYNC)
return -ERANGE;
diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index edb951695b13..34f829845d06 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -1057,9 +1057,6 @@ static int vsc85xx_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- if (cfg.flags)
- return -EINVAL;
-
switch (cfg.tx_type) {
case HWTSTAMP_TX_ONESTEP_SYNC:
one_step = true;
diff --git a/drivers/ptp/ptp_ines.c b/drivers/ptp/ptp_ines.c
index 6c7c2843ba0b..61f47fb9d997 100644
--- a/drivers/ptp/ptp_ines.c
+++ b/drivers/ptp/ptp_ines.c
@@ -338,10 +338,6 @@ static int ines_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- /* reserved for future extensions */
- if (cfg.flags)
- return -EINVAL;
-
switch (cfg.tx_type) {
case HWTSTAMP_TX_OFF:
ts_stat_tx = 0;
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index fcc61c73a666..d3aaab8396ef 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -62,7 +62,7 @@ struct so_timestamping {
/**
* struct hwtstamp_config - %SIOCGHWTSTAMP and %SIOCSHWTSTAMP parameter
*
- * @flags: no flags defined right now, must be zero for %SIOCSHWTSTAMP
+ * @flags: one of HWTSTAMP_FLAGS_*
* @tx_type: one of HWTSTAMP_TX_*
* @rx_filter: one of HWTSTAMP_FILTER_*
*
@@ -78,6 +78,19 @@ struct hwtstamp_config {
int rx_filter;
};
+/* possible values for hwtstamp_config->flags */
+enum hwtstamp_flags {
+ /*
+ * With this flag the user should aware that the PHC index
+ * get/set by syscall is not stable. e.g. the phc index of
+ * bond active interface may changed after failover.
+ */
+ HWTSTAMP_FLAGS_UNSTABLE_PHC = (1<<0),
+
+ /* add new constants above here */
+ __HWTSTAMP_FLAGS_CNT
+};
+
/* possible values for hwtstamp_config->tx_type */
enum hwtstamp_tx_types {
/*
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 1d309a666932..a81c98cfc3db 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -192,9 +192,6 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
- if (cfg.flags) /* reserved for future extensions */
- return -EINVAL;
-
tx_type = cfg.tx_type;
rx_filter = cfg.rx_filter;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next 2/2] Bonding: force user to add HWTSTAMP_FLAGS_UNSTABLE_PHC when get/set HWTSTAMP
2021-12-08 4:42 [PATCH net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAGS_UNSTABLE_PHC Hangbin Liu
2021-12-08 4:42 ` [PATCH net-next 1/2] net_tstamp: add new " Hangbin Liu
@ 2021-12-08 4:42 ` Hangbin Liu
1 sibling, 0 replies; 10+ messages in thread
From: Hangbin Liu @ 2021-12-08 4:42 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S . Miller,
Jakub Kicinski, Richard Cochran, Heiner Kallweit, Hangbin Liu
When there is a failover, the PHC index of bond active interface will
change. This may break the user space program if the author didn't aware.
By setting this flag, the user should aware that the PHC index get/set
by syscall is not stable. And the user space is able to deal with it.
Without this flag, the kernel will reject the request forwarding to
bonding.
Reported-by: Jakub Kicinski <kuba@kernel.org>
Fixes: 94dd016ae538 ("bond: pass get_ts_info and SIOC[SG]HWTSTAMP ioctl to active device")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/bonding/bond_main.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0f39ad2af81c..196fec74944b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4094,6 +4094,7 @@ static int bond_eth_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cm
struct mii_ioctl_data *mii = NULL;
const struct net_device_ops *ops;
struct net_device *real_dev;
+ struct hwtstamp_config cfg;
struct ifreq ifrr;
int res = 0;
@@ -4124,21 +4125,29 @@ static int bond_eth_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cm
break;
case SIOCSHWTSTAMP:
case SIOCGHWTSTAMP:
- rcu_read_lock();
- real_dev = bond_option_active_slave_get_rcu(bond);
- rcu_read_unlock();
- if (real_dev) {
- strscpy_pad(ifrr.ifr_name, real_dev->name, IFNAMSIZ);
- ifrr.ifr_ifru = ifr->ifr_ifru;
+ if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
+ return -EFAULT;
+
+ if (cfg.flags & HWTSTAMP_FLAGS_UNSTABLE_PHC) {
+ rcu_read_lock();
+ real_dev = bond_option_active_slave_get_rcu(bond);
+ rcu_read_unlock();
+ if (real_dev) {
+ strscpy_pad(ifrr.ifr_name, real_dev->name, IFNAMSIZ);
+ ifrr.ifr_ifru = ifr->ifr_ifru;
+
+ ops = real_dev->netdev_ops;
+ if (netif_device_present(real_dev) && ops->ndo_eth_ioctl) {
+ res = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
- ops = real_dev->netdev_ops;
- if (netif_device_present(real_dev) && ops->ndo_eth_ioctl)
- res = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
+ if (!res)
+ ifr->ifr_ifru = ifrr.ifr_ifru;
- if (!res)
- ifr->ifr_ifru = ifrr.ifr_ifru;
+ return res;
+ }
+ }
}
- break;
+ fallthrough;
default:
res = -EOPNOTSUPP;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 4:42 ` [PATCH net-next 1/2] net_tstamp: add new " Hangbin Liu
@ 2021-12-08 6:08 ` Jakub Kicinski
2021-12-08 6:27 ` Hangbin Liu
2021-12-08 15:20 ` Richard Cochran
1 sibling, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-12-08 6:08 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Richard Cochran, Heiner Kallweit
On Wed, 8 Dec 2021 12:42:23 +0800 Hangbin Liu wrote:
> - /* Reserved for future extensions */
> - if (config->flags)
> - return -EINVAL;
Should we do something like:
if (config->flags & ~PHC_DRIVER_IGNORED_FLAGS)
return -EINVAL;
Or whatnot? We still want the drivers to reject bits other than the new
flag.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 6:08 ` Jakub Kicinski
@ 2021-12-08 6:27 ` Hangbin Liu
2021-12-08 22:03 ` Jakub Kicinski
0 siblings, 1 reply; 10+ messages in thread
From: Hangbin Liu @ 2021-12-08 6:27 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Richard Cochran, Heiner Kallweit
On Tue, Dec 07, 2021 at 10:08:14PM -0800, Jakub Kicinski wrote:
> On Wed, 8 Dec 2021 12:42:23 +0800 Hangbin Liu wrote:
> > - /* Reserved for future extensions */
> > - if (config->flags)
> > - return -EINVAL;
>
> Should we do something like:
>
> if (config->flags & ~PHC_DRIVER_IGNORED_FLAGS)
> return -EINVAL;
>
> Or whatnot? We still want the drivers to reject bits other than the new
> flag.
How about just add the check in net_hwtstamp_validate().
I think that should be enough. e.g.
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index a81c98cfc3db..256d2e26487f 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -186,15 +186,27 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
struct hwtstamp_config cfg;
enum hwtstamp_tx_types tx_type;
enum hwtstamp_rx_filters rx_filter;
- int tx_type_valid = 0;
+ enum hwtstamp_flags flags;
int rx_filter_valid = 0;
+ int tx_type_valid = 0;
+ int flags_valid = 0;
if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
return -EFAULT;
+ flags = cfg.flags;
tx_type = cfg.tx_type;
rx_filter = cfg.rx_filter;
+ switch (flags) {
+ case HWTSTAMP_FLAGS_UNSTABLE_PHC:
+ flags_valid = 1;
+ break;
+ case __HWTSTAMP_FLAGS_CNT:
+ /* not a real value */
+ break;
+ }
+
switch (tx_type) {
case HWTSTAMP_TX_OFF:
case HWTSTAMP_TX_ON:
@@ -231,7 +243,7 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
break;
}
- if (!tx_type_valid || !rx_filter_valid)
+ if (!flags_valid || !tx_type_valid || !rx_filter_valid)
return -ERANGE;
return 0;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 4:42 ` [PATCH net-next 1/2] net_tstamp: add new " Hangbin Liu
2021-12-08 6:08 ` Jakub Kicinski
@ 2021-12-08 15:20 ` Richard Cochran
2021-12-09 4:31 ` Hangbin Liu
1 sibling, 1 reply; 10+ messages in thread
From: Richard Cochran @ 2021-12-08 15:20 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Jakub Kicinski, Heiner Kallweit
> diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
> index fcc61c73a666..d3aaab8396ef 100644
> --- a/include/uapi/linux/net_tstamp.h
> +++ b/include/uapi/linux/net_tstamp.h
> @@ -62,7 +62,7 @@ struct so_timestamping {
> /**
> * struct hwtstamp_config - %SIOCGHWTSTAMP and %SIOCSHWTSTAMP parameter
> *
> - * @flags: no flags defined right now, must be zero for %SIOCSHWTSTAMP
> + * @flags: one of HWTSTAMP_FLAGS_*
Nit: should be FLAG (singular) not FLAGS.
1 bit set -> 1 flag enabled
> * @tx_type: one of HWTSTAMP_TX_*
> * @rx_filter: one of HWTSTAMP_FILTER_*
> *
> @@ -78,6 +78,19 @@ struct hwtstamp_config {
> int rx_filter;
> };
>
> +/* possible values for hwtstamp_config->flags */
> +enum hwtstamp_flags {
> + /*
> + * With this flag the user should aware that the PHC index
> + * get/set by syscall is not stable. e.g. the phc index of
> + * bond active interface may changed after failover.
> + */
> + HWTSTAMP_FLAGS_UNSTABLE_PHC = (1<<0),
Can we please find a different name? I see this, and I think,
"unstable ptp hw clock". Nobody would want to use such a clock.
How about HWTSTAMP_FLAG_BONDED_PHC_INDEX ?
> + /* add new constants above here */
> + __HWTSTAMP_FLAGS_CNT
> +};
I guess that the original intent of hwtstamp_config.flags was for user
space to SET flags that it wanted. Now this has become a place for
drivers to return values back. Please make the input/output
distinction clear in the comments.
Thanks,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 6:27 ` Hangbin Liu
@ 2021-12-08 22:03 ` Jakub Kicinski
0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2021-12-08 22:03 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Richard Cochran, Heiner Kallweit
On Wed, 8 Dec 2021 14:27:49 +0800 Hangbin Liu wrote:
> How about just add the check in net_hwtstamp_validate().
Seems like a good idea (modulo the exact implementation)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-08 15:20 ` Richard Cochran
@ 2021-12-09 4:31 ` Hangbin Liu
2021-12-09 21:22 ` Richard Cochran
0 siblings, 1 reply; 10+ messages in thread
From: Hangbin Liu @ 2021-12-09 4:31 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Jakub Kicinski, Heiner Kallweit
On Wed, Dec 08, 2021 at 07:20:22AM -0800, Richard Cochran wrote:
> > +/* possible values for hwtstamp_config->flags */
> > +enum hwtstamp_flags {
> > + /*
> > + * With this flag the user should aware that the PHC index
> > + * get/set by syscall is not stable. e.g. the phc index of
> > + * bond active interface may changed after failover.
> > + */
> > + HWTSTAMP_FLAGS_UNSTABLE_PHC = (1<<0),
>
> Can we please find a different name? I see this, and I think,
> "unstable ptp hw clock". Nobody would want to use such a clock.
>
> How about HWTSTAMP_FLAG_BONDED_PHC_INDEX ?
Thanks, this one looks better.
>
> > + /* add new constants above here */
> > + __HWTSTAMP_FLAGS_CNT
> > +};
>
> I guess that the original intent of hwtstamp_config.flags was for user
> space to SET flags that it wanted.
> Now this has become a place for drivers to return values back.
I think it's a flag that when uses want phc index of bond.
There is no affect for other drivers. It only affect bond interfaces.
When this flag set, it means users want to get the info from bond.
Do I missed something?
> Please make the input/output distinction clear in the comments.
Yes, with the flag name changed to HWTSTAMP_FLAG_BONDED_PHC_INDEX.
The comments also need update.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-09 4:31 ` Hangbin Liu
@ 2021-12-09 21:22 ` Richard Cochran
2021-12-09 21:30 ` Richard Cochran
0 siblings, 1 reply; 10+ messages in thread
From: Richard Cochran @ 2021-12-09 21:22 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Jakub Kicinski, Heiner Kallweit
On Thu, Dec 09, 2021 at 12:31:30PM +0800, Hangbin Liu wrote:
> On Wed, Dec 08, 2021 at 07:20:22AM -0800, Richard Cochran wrote:
> > I guess that the original intent of hwtstamp_config.flags was for user
> > space to SET flags that it wanted.
> > Now this has become a place for drivers to return values back.
>
> I think it's a flag that when uses want phc index of bond.
> There is no affect for other drivers. It only affect bond interfaces.
> When this flag set, it means users want to get the info from bond.
>
> Do I missed something?
No, I simply mean that the input/output direction of the bit in the
flags should be clear.
- User space will not set this bit, only read it.
- Drivers may set this bit, but if user sets it, it is an error.
Thanks,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAGS_UNSTABLE_PHC
2021-12-09 21:22 ` Richard Cochran
@ 2021-12-09 21:30 ` Richard Cochran
0 siblings, 0 replies; 10+ messages in thread
From: Richard Cochran @ 2021-12-09 21:30 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S . Miller, Jakub Kicinski, Heiner Kallweit
On Thu, Dec 09, 2021 at 01:22:58PM -0800, Richard Cochran wrote:
> On Thu, Dec 09, 2021 at 12:31:30PM +0800, Hangbin Liu wrote:
> > On Wed, Dec 08, 2021 at 07:20:22AM -0800, Richard Cochran wrote:
> > > I guess that the original intent of hwtstamp_config.flags was for user
> > > space to SET flags that it wanted.
> > > Now this has become a place for drivers to return values back.
> >
> > I think it's a flag that when uses want phc index of bond.
> > There is no affect for other drivers. It only affect bond interfaces.
> > When this flag set, it means users want to get the info from bond.
> >
> > Do I missed something?
>
> No, I simply mean that the input/output direction of the bit in the
> flags should be clear.
>
> - User space will not set this bit, only read it.
> - Drivers may set this bit, but if user sets it, it is an error.
Oh, I am confused. Your patch does this:
- if user sets bit, then return bonded index
- otherwise, return EOPNOTSUPP
That is fine with me.
Thanks,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-09 21:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-08 4:42 [PATCH net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAGS_UNSTABLE_PHC Hangbin Liu
2021-12-08 4:42 ` [PATCH net-next 1/2] net_tstamp: add new " Hangbin Liu
2021-12-08 6:08 ` Jakub Kicinski
2021-12-08 6:27 ` Hangbin Liu
2021-12-08 22:03 ` Jakub Kicinski
2021-12-08 15:20 ` Richard Cochran
2021-12-09 4:31 ` Hangbin Liu
2021-12-09 21:22 ` Richard Cochran
2021-12-09 21:30 ` Richard Cochran
2021-12-08 4:42 ` [PATCH net-next 2/2] Bonding: force user to add HWTSTAMP_FLAGS_UNSTABLE_PHC when get/set HWTSTAMP Hangbin Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).