All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
@ 2021-12-09 10:24 Hangbin Liu
  2021-12-09 10:24 ` [PATCHv2 net-next 1/2] net_tstamp: add new " Hangbin Liu
  2021-12-09 10:24 ` [PATCHv2 net-next 2/2] Bonding: force user to add HWTSTAMP_FLAG_BONDED_PHC_INDEX when get/set HWTSTAMP Hangbin Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2021-12-09 10:24 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_FLAG_BONDED_PHC_INDEX.
When user want to get bond active interface's PHC, they need to add this flag
and aware the PHC index may changed.

v2: rename the flag to HWTSTAMP_FLAG_BONDED_PHC_INDEX

Hangbin Liu (2):
  net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
  Bonding: force user to add HWTSTAMP_FLAG_BONDED_PHC_INDEX 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               | 16 ++++++++-
 net/core/dev_ioctl.c                          | 19 ++++++++---
 41 files changed, 50 insertions(+), 160 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
  2021-12-09 10:24 [PATCHv2 net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAG_BONDED_PHC_INDEX Hangbin Liu
@ 2021-12-09 10:24 ` Hangbin Liu
  2021-12-09 21:27   ` Richard Cochran
  2021-12-09 21:33   ` Richard Cochran
  2021-12-09 10:24 ` [PATCHv2 net-next 2/2] Bonding: force user to add HWTSTAMP_FLAG_BONDED_PHC_INDEX when get/set HWTSTAMP Hangbin Liu
  1 sibling, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2021-12-09 10:24 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_FLAG_BONDED_PHC_INDEX.
When the user wants to get the 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 are removed. Only
the checking in net_hwtstamp_validate() is kept.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

---
v2: Keep the flag validation check in net_hwtstamp_validate()
    Rename the flag to HWTSTAMP_FLAG_BONDED_PHC_INDEX
---
 .../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               | 16 +++++++++++++++-
 net/core/dev_ioctl.c                          | 19 ++++++++++++++-----
 40 files changed, 29 insertions(+), 148 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..1cf3b43308ac 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_FLAG_*
  * @tx_type:	one of HWTSTAMP_TX_*
  * @rx_filter:	one of HWTSTAMP_FILTER_*
  *
@@ -78,6 +78,20 @@ struct hwtstamp_config {
 	int rx_filter;
 };
 
+/* possible values for hwtstamp_config->flags */
+enum hwtstamp_flags {
+	/*
+	 * With this flag, the user could get bond active interface's
+	 * PHC index. Note this PHC index is not stable as when there
+	 * is a failover, the bond active interface will be changed, so
+	 * will be the PHC index.
+	 */
+	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (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..10ac5457dcbc 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -186,18 +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 flag;
 	int rx_filter_valid = 0;
+	int tx_type_valid = 0;
+	int flag_valid = 0;
 
 	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
 		return -EFAULT;
 
-	if (cfg.flags) /* reserved for future extensions */
-		return -EINVAL;
-
+	flag = cfg.flags;
 	tx_type = cfg.tx_type;
 	rx_filter = cfg.rx_filter;
 
+	switch (flag) {
+	case HWTSTAMP_FLAG_BONDED_PHC_INDEX:
+		flag_valid = 1;
+		break;
+	case __HWTSTAMP_FLAGS_CNT:
+		/* not a real value */
+		break;
+	}
+
 	switch (tx_type) {
 	case HWTSTAMP_TX_OFF:
 	case HWTSTAMP_TX_ON:
@@ -234,7 +243,7 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
 		break;
 	}
 
-	if (!tx_type_valid || !rx_filter_valid)
+	if (!flag_valid || !tx_type_valid || !rx_filter_valid)
 		return -ERANGE;
 
 	return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 net-next 2/2] Bonding: force user to add HWTSTAMP_FLAG_BONDED_PHC_INDEX when get/set HWTSTAMP
  2021-12-09 10:24 [PATCHv2 net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAG_BONDED_PHC_INDEX Hangbin Liu
  2021-12-09 10:24 ` [PATCHv2 net-next 1/2] net_tstamp: add new " Hangbin Liu
@ 2021-12-09 10:24 ` Hangbin Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2021-12-09 10:24 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 be
changed. 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>

---
v2: change the flag name to HWTSTAMP_FLAG_BONDED_PHC_INDEX
---
 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..268190a624e0 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_FLAG_BONDED_PHC_INDEX) {
+			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] 6+ messages in thread

* Re: [PATCHv2 net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
  2021-12-09 10:24 ` [PATCHv2 net-next 1/2] net_tstamp: add new " Hangbin Liu
@ 2021-12-09 21:27   ` Richard Cochran
  2021-12-09 21:33   ` Richard Cochran
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Cochran @ 2021-12-09 21:27 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 06:24:48PM +0800, Hangbin Liu wrote:
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index 1d309a666932..10ac5457dcbc 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -186,18 +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 flag;
>  	int rx_filter_valid = 0;
> +	int tx_type_valid = 0;
> +	int flag_valid = 0;
>  
>  	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
>  		return -EFAULT;
>  
> -	if (cfg.flags) /* reserved for future extensions */
> -		return -EINVAL;
> -
> +	flag = cfg.flags;
>  	tx_type = cfg.tx_type;
>  	rx_filter = cfg.rx_filter;
>  
> +	switch (flag) {
> +	case HWTSTAMP_FLAG_BONDED_PHC_INDEX:
> +		flag_valid = 1;

This isn't correct.  You need to use a bitwise test.

Thanks,
Richard


> +		break;
> +	case __HWTSTAMP_FLAGS_CNT:
> +		/* not a real value */
> +		break;
> +	}
> +
>  	switch (tx_type) {
>  	case HWTSTAMP_TX_OFF:
>  	case HWTSTAMP_TX_ON:
> @@ -234,7 +243,7 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
>  		break;
>  	}
>  
> -	if (!tx_type_valid || !rx_filter_valid)
> +	if (!flag_valid || !tx_type_valid || !rx_filter_valid)
>  		return -ERANGE;
>  
>  	return 0;
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
  2021-12-09 10:24 ` [PATCHv2 net-next 1/2] net_tstamp: add new " Hangbin Liu
  2021-12-09 21:27   ` Richard Cochran
@ 2021-12-09 21:33   ` Richard Cochran
  2021-12-09 21:38     ` Richard Cochran
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Cochran @ 2021-12-09 21:33 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 06:24:48PM +0800, Hangbin Liu wrote:

> +/* possible values for hwtstamp_config->flags */
> +enum hwtstamp_flags {
> +	/*
> +	 * With this flag, the user could get bond active interface's
> +	 * PHC index. Note this PHC index is not stable as when there
> +	 * is a failover, the bond active interface will be changed, so
> +	 * will be the PHC index.
> +	 */
> +	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
> +
> +	/* add new constants above here */
> +	__HWTSTAMP_FLAGS_CNT
> +};

I think this shouldn't be an enumerated type, but rather simply a bit
field of independent options.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 net-next 1/2] net_tstamp: add new flag HWTSTAMP_FLAG_BONDED_PHC_INDEX
  2021-12-09 21:33   ` Richard Cochran
@ 2021-12-09 21:38     ` Richard Cochran
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Cochran @ 2021-12-09 21:38 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:33:47PM -0800, Richard Cochran wrote:
> On Thu, Dec 09, 2021 at 06:24:48PM +0800, Hangbin Liu wrote:
> 
> > +/* possible values for hwtstamp_config->flags */
> > +enum hwtstamp_flags {
> > +	/*
> > +	 * With this flag, the user could get bond active interface's
> > +	 * PHC index. Note this PHC index is not stable as when there
> > +	 * is a failover, the bond active interface will be changed, so
> > +	 * will be the PHC index.
> > +	 */
> > +	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
> > +
> > +	/* add new constants above here */
> > +	__HWTSTAMP_FLAGS_CNT
> > +};
> 
> I think this shouldn't be an enumerated type, but rather simply a bit
> field of independent options.

Ok, it can be an enum (to be like the other fields in this header) but
still the bits need to be independent of each other.

IOW, you should drop __HWTSTAMP_FLAGS_CNT and instead use a mask of
valid bits.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-09 21:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 10:24 [PATCHv2 net-next 0/2] net: add new hwtstamp flag HWTSTAMP_FLAG_BONDED_PHC_INDEX Hangbin Liu
2021-12-09 10:24 ` [PATCHv2 net-next 1/2] net_tstamp: add new " Hangbin Liu
2021-12-09 21:27   ` Richard Cochran
2021-12-09 21:33   ` Richard Cochran
2021-12-09 21:38     ` Richard Cochran
2021-12-09 10:24 ` [PATCHv2 net-next 2/2] Bonding: force user to add HWTSTAMP_FLAG_BONDED_PHC_INDEX when get/set HWTSTAMP Hangbin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.