netdev - Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow hwstamping on the master port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221110125322.c436jqyplxuzdvcl@skbuf>
Date:   Thu, 10 Nov 2022 14:53:22 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Fabio Estevam <festevam@...il.com>
Cc:     kuba@...nel.org, andrew@...n.ch, netdev@...r.kernel.org,
        Steffen Bätz <steffen@...osonix.de>,
        Fabio Estevam <festevam@...x.de>
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow hwstamping on the
 master port

On Thu, Nov 10, 2022 at 09:43:45AM -0300, Fabio Estevam wrote:
> From: Steffen Bätz <steffen@...osonix.de>
> 
> Currently, it is not possible to run SIOCGHWTSTAMP or SIOCSHWTSTAMP
> ioctls on the dsa master interface if the port_hwtstamp_set/get()
> hooks are present, as implemented in net/dsa/master.c:
> 
> static int dsa_master_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> {
> ...
> 	case SIOCGHWTSTAMP:
> 	case SIOCSHWTSTAMP:
> 		/* Deny PTP operations on master if there is at least one
> 		 * switch in the tree that is PTP capable.
> 		 */
> 		list_for_each_entry(dp, &dst->ports, list)
> 			if (dp->ds->ops->port_hwtstamp_get ||
> 			    dp->ds->ops->port_hwtstamp_set)
> 				return -EBUSY;
> 		break;
> 	}
> 
> Even if the hwtstamping functionality is disabled in the mv88e6xxx driver
> by not setting CONFIG_NET_DSA_MV88E6XXX_PTP, the functions port_hwtstamp_set()
> port_hwtstamp_get() are still present due to their stub declarations.
> 
> Fix this problem, by removing the stub declarations and guarding these
> functions wih CONFIG_NET_DSA_MV88E6XXX_PTP.
> 
> Without this change:
> 
>  # hwstamp_ctl -i eth0
> SIOCGHWTSTAMP failed: Device or resource busy
> 
> With the change applied, it is possible to set and get the timestamping
> options:
> 
>  # hwstamp_ctl -i eth0
> current settings:
> tx_type 0
> rx_filter 0
> 
>  # hwstamp_ctl -i eth0 -r 1 -t 1
> current settings:
> tx_type 0
> rx_filter 0
> new settings:
> tx_type 1
> rx_filter 1
> 
> Tested on a custom i.MX8MN board with a 88E6320 switch.
> 
> Signed-off-by: Steffen Bätz <steffen@...osonix.de>
> Signed-off-by: Fabio Estevam <festevam@...x.de>
> ---

NACK.

Please extend dsa_master_ioctl() to "see through" the dp->ds->ops->port_hwtstamp_get()
pointer, similar to what dsa_port_can_configure_learning() does. Create
a fake struct ifreq, call port_hwtstamp_get(), see if it returned
-EOPNOTSUPP, and wrap that into a dsa_port_supports_hwtstamp() function,
and call that instead of the current simplistic checks for the function
pointers.