[PATCH net-next 0/3] sfc: add support for PTP over IPv6 and 802.3

[PATCH net-next 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

Most recent cards (8000 series and newer) had enough hardware support
for this, but it was not enabled in the driver. The transmission of PTP
packets over these protocols was already added in commit bd4a2697e5e2
("sfc: use hardware tx timestamps for more than PTP"), but receiving
them was already unsupported so synchronization didn't happen.

These patches add support for timestamping received packets over
IPv6/UPD and IEEE802.3.

Íñigo Huguet (3):
  sfc: allow more flexible way of adding filters for PTP
  sfc: support PTP over IPv6/UDP
  sfc: support PTP over Ethernet

 drivers/net/ethernet/sfc/filter.h |  22 +++++
 drivers/net/ethernet/sfc/ptp.c    | 132 ++++++++++++++++++++----------
 2 files changed, 110 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH net-next 1/3] sfc: allow more flexible way of adding filters for PTP

[Íñigo Huguet]

In preparation for the support of PTP over IPv6/UDP and Ethernet in next
patches, allow a more flexible way of adding and removing RX filters for
PTP. Right now, only 2 filters are allowed, which are the ones needed
for PTP over IPv4/UDP.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 72 ++++++++++++++++------------------
 1 file changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 10ad0b93d283..bdf6c09cfac5 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
-#define PTP_EVENT_PORT		319
-#define PTP_GENERAL_PORT	320
+#define PTP_RXFILTERS_LEN	2
+
+#define PTP_ADDRESS		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_EVENT_PORT		htons(319)
+#define PTP_GENERAL_PORT	htons(320)
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -224,9 +226,8 @@ struct efx_ptp_timeset {
  * @work: Work task
  * @reset_required: A serious error has occurred and the PTP task needs to be
  *                  reset (disable, enable).
- * @rxfilter_event: Receive filter when operating
- * @rxfilter_general: Receive filter when operating
- * @rxfilter_installed: Receive filter installed
+ * @rxfilters: Receive filters when operating
+ * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
  * @config: Current timestamp configuration
  * @enabled: PTP operation enabled
  * @mode: Mode in which PTP operating (PTP version)
@@ -295,9 +296,8 @@ struct efx_ptp_data {
 	struct workqueue_struct *workwq;
 	struct work_struct work;
 	bool reset_required;
-	u32 rxfilter_event;
-	u32 rxfilter_general;
-	bool rxfilter_installed;
+	u32 rxfilters[PTP_RXFILTERS_LEN];
+	size_t rxfilters_count;
 	struct hwtstamp_config config;
 	bool enabled;
 	unsigned int mode;
@@ -1290,61 +1290,55 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	if (ptp->rxfilter_installed) {
-		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_general);
+	while (ptp->rxfilters_count) {
+		ptp->rxfilters_count--;
 		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_event);
-		ptp->rxfilter_installed = false;
+					  ptp->rxfilters[ptp->rxfilters_count]);
 	}
 }
 
-static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 	struct efx_filter_spec rxfilter;
 	int rc;
 
-	if (!ptp->channel || ptp->rxfilter_installed)
-		return 0;
-
-	/* Must filter on both event and general ports to ensure
-	 * that there is no packet re-ordering.
-	 */
 	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
 			   efx_rx_queue_index(
 				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_EVENT_PORT));
-	if (rc != 0)
-		return rc;
+
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDRESS, port);
 
 	rc = efx_filter_insert_filter(efx, &rxfilter, true);
 	if (rc < 0)
 		return rc;
-	ptp->rxfilter_event = rc;
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_GENERAL_PORT));
-	if (rc != 0)
+	ptp->rxfilters[ptp->rxfilters_count] = rc;
+	ptp->rxfilters_count++;
+
+	return 0;
+}
+
+static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
+	int rc;
+
+	if (!ptp->channel || ptp->rxfilters_count)
+		return 0;
+
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
+	if (rc < 0)
 		goto fail;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
 	if (rc < 0)
 		goto fail;
-	ptp->rxfilter_general = rc;
 
-	ptp->rxfilter_installed = true;
 	return 0;
 
 fail:
-	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-				  ptp->rxfilter_event);
+	efx_ptp_remove_multicast_filters(efx);
 	return rc;
 }
 
-- 
2.34.1

Re: [PATCH net-next 1/3] sfc: allow more flexible way of adding filters for PTP

[Edward Cree]

On 09/08/2022 10:20, Íñigo Huguet wrote:
> In preparation for the support of PTP over IPv6/UDP and Ethernet in next
> patches, allow a more flexible way of adding and removing RX filters for
> PTP. Right now, only 2 filters are allowed, which are the ones needed
> for PTP over IPv4/UDP.
> 
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>

Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

(But you'll probably need to wait and respin when net-next reopens.)

[PATCH net-next 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
PTP") added support for hardware timestamping on TX for cards of the
8000 series and newer, in an effort to provide support for other
transports other than IPv4/UDP.

However, timestamping was still not working on RX for these other
transports. This patch add support for PTP over IPv6/UDP.

Tested: sync as master and as slave is correct using ptp4l from linuxptp
package, both with IPv4 and IPv6.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/filter.h | 22 +++++++++++
 drivers/net/ethernet/sfc/ptp.c    | 61 ++++++++++++++++++++++++-------
 2 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/sfc/filter.h b/drivers/net/ethernet/sfc/filter.h
index 4d928839d292..be72e71da027 100644
--- a/drivers/net/ethernet/sfc/filter.h
+++ b/drivers/net/ethernet/sfc/filter.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/in6.h>
 #include <asm/byteorder.h>
 
 /**
@@ -223,6 +224,27 @@ efx_filter_set_ipv4_local(struct efx_filter_spec *spec, u8 proto,
 	return 0;
 }
 
+/**
+ * efx_filter_set_ipv6_local - specify IPv6 host, transport protocol and port
+ * @spec: Specification to initialise
+ * @proto: Transport layer protocol number
+ * @host: Local host address (network byte order)
+ * @port: Local port (network byte order)
+ */
+static inline int
+efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
+			  const struct in6_addr *host, __be16 port)
+{
+	spec->match_flags |=
+		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
+		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT;
+	spec->ether_type = htons(ETH_P_IPV6);
+	spec->ip_proto = proto;
+	memcpy(spec->loc_host, host, sizeof(spec->loc_host));
+	spec->loc_port = port;
+	return 0;
+}
+
 /**
  * efx_filter_set_ipv4_full - specify IPv4 hosts, transport protocol and ports
  * @spec: Specification to initialise
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index bdf6c09cfac5..1e6a70fce77a 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	2
+#define PTP_RXFILTERS_LEN	4
 
-#define PTP_ADDRESS		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_ADDR_IPV4		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+				0, 0x01, 0x81}		/* ff0e::181 */
 #define PTP_EVENT_PORT		htons(319)
 #define PTP_GENERAL_PORT	htons(320)
 
@@ -1297,28 +1299,46 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 	}
 }
 
-static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
+static inline void efx_ptp_init_filter(struct efx_nic *efx,
+				       struct efx_filter_spec *rxfilter)
 {
-	struct efx_ptp_data *ptp = efx->ptp_data;
-	struct efx_filter_spec rxfilter;
-	int rc;
-
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
+	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
+			   efx_rx_queue_index(efx_channel_get_rx_queue(
+					      efx->ptp_data->channel)));
+}
 
-	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDRESS, port);
+static inline int efx_ptp_insert_filter(struct efx_nic *efx,
+					struct efx_filter_spec *rxfilter)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	int rc = efx_filter_insert_filter(efx, rxfilter, true);
 	if (rc < 0)
 		return rc;
-
 	ptp->rxfilters[ptp->rxfilters_count] = rc;
 	ptp->rxfilters_count++;
-
 	return 0;
 }
 
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
+{
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDR_IPV4, port);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
+static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, __be16 port)
+{
+	const struct in6_addr addr = {{PTP_ADDR_IPV6}};
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv6_local(&rxfilter, IPPROTO_UDP, &addr, port);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1335,6 +1355,19 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 	if (rc < 0)
 		goto fail;
 
+	/* if the NIC supports hw timestamps by the MAC, we can support
+	 * PTP over IPv6
+	 */
+	if (efx_ptp_use_mac_tx_timestamps(efx)) {
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
+		if (rc < 0)
+			goto fail;
+
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
+		if (rc < 0)
+			goto fail;
+	}
+
 	return 0;
 
 fail:
-- 
2.34.1

Re: [PATCH net-next 2/3] sfc: support PTP over IPv6/UDP

[Edward Cree]

On 09/08/2022 10:20, Íñigo Huguet wrote:
> @@ -1297,28 +1299,46 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
>  	}
>  }
>  
> -static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
> +static inline void efx_ptp_init_filter(struct efx_nic *efx,
> +				       struct efx_filter_spec *rxfilter)
>  {
> -	struct efx_ptp_data *ptp = efx->ptp_data;
> -	struct efx_filter_spec rxfilter;
> -	int rc;
> -
> -	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
> -			   efx_rx_queue_index(
> -				   efx_channel_get_rx_queue(ptp->channel)));
> +	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
> +			   efx_rx_queue_index(efx_channel_get_rx_queue(
> +					      efx->ptp_data->channel)));

This indentation is misleading, it makes it look as though
 efx->ptp_data->channel is an argument to efx_rx_queue_index().
Please just use a local (how about struct efx_rx_queue *?).

Otherwise looks good.

-ed

[PATCH net-next 3/3] sfc: support PTP over Ethernet

[Íñigo Huguet]

The previous patch add support for PTP over IPv6/UDP (only for 8000
series and newer) and this one add support for PTP over 802.3.

Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
and IPv6 still works fine.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 1e6a70fce77a..7e8390a5a757 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,13 +118,14 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	4
+#define PTP_RXFILTERS_LEN	5
 
 #define PTP_ADDR_IPV4		htonl(0xe0000181)	/* 224.0.1.129 */
 #define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 				0, 0x01, 0x81}		/* ff0e::181 */
 #define PTP_EVENT_PORT		htons(319)
 #define PTP_GENERAL_PORT	htons(320)
+#define PTP_ADDR_ETHER		{0x01, 0x1b, 0x19, 0, 0, 0} /* 01-1B-19-00-00-00 */
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -1339,6 +1340,18 @@ static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, __be16 port)
 	return efx_ptp_insert_filter(efx, &rxfilter);
 }
 
+static int efx_ptp_insert_eth_filter(struct efx_nic *efx)
+{
+	const u8 addr[ETH_ALEN] = PTP_ADDR_ETHER;
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_eth_local(&rxfilter, EFX_FILTER_VID_UNSPEC, addr);
+	rxfilter.match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+	rxfilter.ether_type = htons(ETH_P_1588);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1356,7 +1369,7 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		goto fail;
 
 	/* if the NIC supports hw timestamps by the MAC, we can support
-	 * PTP over IPv6
+	 * PTP over IPv6 and Ethernet
 	 */
 	if (efx_ptp_use_mac_tx_timestamps(efx)) {
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
@@ -1366,6 +1379,10 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
 		if (rc < 0)
 			goto fail;
+
+		rc = efx_ptp_insert_eth_filter(efx);
+		if (rc < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH net-next 3/3] sfc: support PTP over Ethernet

[Edward Cree]

On 09/08/2022 10:20, Íñigo Huguet wrote:
> The previous patch add support for PTP over IPv6/UDP (only for 8000
> series and newer) and this one add support for PTP over 802.3.
> 
> Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
> and IPv6 still works fine.
> 
> Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>

Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

[PATCH net-next v2 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

Most recent cards (8000 series and newer) had enough hardware support
for this, but it was not enabled in the driver. The transmission of PTP
packets over these protocols was already added in commit bd4a2697e5e2
("sfc: use hardware tx timestamps for more than PTP"), but receiving
them was already unsupported so synchronization didn't happen.

These patches add support for timestamping received packets over
IPv6/UPD and IEEE802.3.

v2: fixed weird indentation in efx_ptp_init_filter

Íñigo Huguet (3):
  sfc: allow more flexible way of adding filters for PTP
  sfc: support PTP over IPv6/UDP
  sfc: support PTP over Ethernet

 drivers/net/ethernet/sfc/filter.h |  22 +++++
 drivers/net/ethernet/sfc/ptp.c    | 134 ++++++++++++++++++++----------
 2 files changed, 112 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH net-next v2 1/3] sfc: allow more flexible way of adding filters for PTP

[Íñigo Huguet]

In preparation for the support of PTP over IPv6/UDP and Ethernet in next
patches, allow a more flexible way of adding and removing RX filters for
PTP. Right now, only 2 filters are allowed, which are the ones needed
for PTP over IPv4/UDP.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 72 ++++++++++++++++------------------
 1 file changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 10ad0b93d283..bdf6c09cfac5 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
-#define PTP_EVENT_PORT		319
-#define PTP_GENERAL_PORT	320
+#define PTP_RXFILTERS_LEN	2
+
+#define PTP_ADDRESS		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_EVENT_PORT		htons(319)
+#define PTP_GENERAL_PORT	htons(320)
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -224,9 +226,8 @@ struct efx_ptp_timeset {
  * @work: Work task
  * @reset_required: A serious error has occurred and the PTP task needs to be
  *                  reset (disable, enable).
- * @rxfilter_event: Receive filter when operating
- * @rxfilter_general: Receive filter when operating
- * @rxfilter_installed: Receive filter installed
+ * @rxfilters: Receive filters when operating
+ * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
  * @config: Current timestamp configuration
  * @enabled: PTP operation enabled
  * @mode: Mode in which PTP operating (PTP version)
@@ -295,9 +296,8 @@ struct efx_ptp_data {
 	struct workqueue_struct *workwq;
 	struct work_struct work;
 	bool reset_required;
-	u32 rxfilter_event;
-	u32 rxfilter_general;
-	bool rxfilter_installed;
+	u32 rxfilters[PTP_RXFILTERS_LEN];
+	size_t rxfilters_count;
 	struct hwtstamp_config config;
 	bool enabled;
 	unsigned int mode;
@@ -1290,61 +1290,55 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	if (ptp->rxfilter_installed) {
-		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_general);
+	while (ptp->rxfilters_count) {
+		ptp->rxfilters_count--;
 		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_event);
-		ptp->rxfilter_installed = false;
+					  ptp->rxfilters[ptp->rxfilters_count]);
 	}
 }
 
-static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 	struct efx_filter_spec rxfilter;
 	int rc;
 
-	if (!ptp->channel || ptp->rxfilter_installed)
-		return 0;
-
-	/* Must filter on both event and general ports to ensure
-	 * that there is no packet re-ordering.
-	 */
 	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
 			   efx_rx_queue_index(
 				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_EVENT_PORT));
-	if (rc != 0)
-		return rc;
+
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDRESS, port);
 
 	rc = efx_filter_insert_filter(efx, &rxfilter, true);
 	if (rc < 0)
 		return rc;
-	ptp->rxfilter_event = rc;
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_GENERAL_PORT));
-	if (rc != 0)
+	ptp->rxfilters[ptp->rxfilters_count] = rc;
+	ptp->rxfilters_count++;
+
+	return 0;
+}
+
+static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
+	int rc;
+
+	if (!ptp->channel || ptp->rxfilters_count)
+		return 0;
+
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
+	if (rc < 0)
 		goto fail;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
 	if (rc < 0)
 		goto fail;
-	ptp->rxfilter_general = rc;
 
-	ptp->rxfilter_installed = true;
 	return 0;
 
 fail:
-	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-				  ptp->rxfilter_event);
+	efx_ptp_remove_multicast_filters(efx);
 	return rc;
 }
 
-- 
2.34.1

Re: [PATCH net-next v2 1/3] sfc: allow more flexible way of adding filters for PTP

[kernel test robot]

Hi "Íñigo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/igo-Huguet/sfc-allow-more-flexible-way-of-adding-filters-for-PTP/20220819-172020
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 268603d79cc48dba671e9caf108fab32315b86a2
config: riscv-randconfig-s032-20220820 (https://download.01.org/0day-ci/archive/20220820/202208200349.DAfcHJLZ-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/b2ecd6ff1d511bc31dbb222211226ce141b0852b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review igo-Huguet/sfc-allow-more-flexible-way-of-adding-filters-for-PTP/20220819-172020
        git checkout b2ecd6ff1d511bc31dbb222211226ce141b0852b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash drivers/net/ethernet/sfc/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
   WARNING: invalid argument to '-march': '_zihintpause'
>> drivers/net/ethernet/sfc/ptp.c:1636:39: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 [usertype] @@
   drivers/net/ethernet/sfc/ptp.c:1636:39: sparse:     expected unsigned short [usertype] val
   drivers/net/ethernet/sfc/ptp.c:1636:39: sparse:     got restricted __be16 [usertype]
   drivers/net/ethernet/sfc/ptp.c:1636:39: sparse: sparse: cast from restricted __be16
   drivers/net/ethernet/sfc/ptp.c:1636:39: sparse: sparse: cast from restricted __be16
   drivers/net/ethernet/sfc/ptp.c:1697:58: sparse: sparse: restricted __be16 degrades to integer
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 [usertype] @@
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse:     expected unsigned short [usertype] val
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse:     got restricted __be16 [usertype]
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse: sparse: cast from restricted __be16
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse: sparse: cast from restricted __be16

vim +1636 drivers/net/ethernet/sfc/ptp.c

7c236c43b83822 Stuart Hodgson 2012-09-03  1620  
7c236c43b83822 Stuart Hodgson 2012-09-03  1621  /* Determine whether this packet should be processed by the PTP module
7c236c43b83822 Stuart Hodgson 2012-09-03  1622   * or transmitted conventionally.
7c236c43b83822 Stuart Hodgson 2012-09-03  1623   */
7c236c43b83822 Stuart Hodgson 2012-09-03  1624  bool efx_ptp_is_ptp_tx(struct efx_nic *efx, struct sk_buff *skb)
7c236c43b83822 Stuart Hodgson 2012-09-03  1625  {
7c236c43b83822 Stuart Hodgson 2012-09-03  1626  	return efx->ptp_data &&
7c236c43b83822 Stuart Hodgson 2012-09-03  1627  		efx->ptp_data->enabled &&
7c236c43b83822 Stuart Hodgson 2012-09-03  1628  		skb->len >= PTP_MIN_LENGTH &&
7c236c43b83822 Stuart Hodgson 2012-09-03  1629  		skb->len <= MC_CMD_PTP_IN_TRANSMIT_PACKET_MAXNUM &&
7c236c43b83822 Stuart Hodgson 2012-09-03  1630  		likely(skb->protocol == htons(ETH_P_IP)) &&
e5a498e943fbc4 Ben Hutchings  2013-12-06  1631  		skb_transport_header_was_set(skb) &&
e5a498e943fbc4 Ben Hutchings  2013-12-06  1632  		skb_network_header_len(skb) >= sizeof(struct iphdr) &&
7c236c43b83822 Stuart Hodgson 2012-09-03  1633  		ip_hdr(skb)->protocol == IPPROTO_UDP &&
e5a498e943fbc4 Ben Hutchings  2013-12-06  1634  		skb_headlen(skb) >=
e5a498e943fbc4 Ben Hutchings  2013-12-06  1635  		skb_transport_offset(skb) + sizeof(struct udphdr) &&
7c236c43b83822 Stuart Hodgson 2012-09-03 @1636  		udp_hdr(skb)->dest == htons(PTP_EVENT_PORT);
7c236c43b83822 Stuart Hodgson 2012-09-03  1637  }
7c236c43b83822 Stuart Hodgson 2012-09-03  1638  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH net-next v2 1/3] sfc: allow more flexible way of adding filters for PTP

[kernel test robot]

Hi "Íñigo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/igo-Huguet/sfc-allow-more-flexible-way-of-adding-filters-for-PTP/20220819-172020
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 268603d79cc48dba671e9caf108fab32315b86a2
config: sparc-randconfig-s043-20220820 (https://download.01.org/0day-ci/archive/20220820/202208200516.VBfWImxe-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/b2ecd6ff1d511bc31dbb222211226ce141b0852b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review igo-Huguet/sfc-allow-more-flexible-way-of-adding-filters-for-PTP/20220819-172020
        git checkout b2ecd6ff1d511bc31dbb222211226ce141b0852b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc SHELL=/bin/bash drivers/net/ethernet/sfc/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/sfc/ptp.c:1636:39: sparse: sparse: cast from restricted __be16
>> drivers/net/ethernet/sfc/ptp.c:1697:58: sparse: sparse: restricted __be16 degrades to integer
   drivers/net/ethernet/sfc/ptp.c:1736:36: sparse: sparse: cast from restricted __be16

vim +1636 drivers/net/ethernet/sfc/ptp.c

7c236c43b838221 Stuart Hodgson  2012-09-03  1620  
7c236c43b838221 Stuart Hodgson  2012-09-03  1621  /* Determine whether this packet should be processed by the PTP module
7c236c43b838221 Stuart Hodgson  2012-09-03  1622   * or transmitted conventionally.
7c236c43b838221 Stuart Hodgson  2012-09-03  1623   */
7c236c43b838221 Stuart Hodgson  2012-09-03  1624  bool efx_ptp_is_ptp_tx(struct efx_nic *efx, struct sk_buff *skb)
7c236c43b838221 Stuart Hodgson  2012-09-03  1625  {
7c236c43b838221 Stuart Hodgson  2012-09-03  1626  	return efx->ptp_data &&
7c236c43b838221 Stuart Hodgson  2012-09-03  1627  		efx->ptp_data->enabled &&
7c236c43b838221 Stuart Hodgson  2012-09-03  1628  		skb->len >= PTP_MIN_LENGTH &&
7c236c43b838221 Stuart Hodgson  2012-09-03  1629  		skb->len <= MC_CMD_PTP_IN_TRANSMIT_PACKET_MAXNUM &&
7c236c43b838221 Stuart Hodgson  2012-09-03  1630  		likely(skb->protocol == htons(ETH_P_IP)) &&
e5a498e943fbc49 Ben Hutchings   2013-12-06  1631  		skb_transport_header_was_set(skb) &&
e5a498e943fbc49 Ben Hutchings   2013-12-06  1632  		skb_network_header_len(skb) >= sizeof(struct iphdr) &&
7c236c43b838221 Stuart Hodgson  2012-09-03  1633  		ip_hdr(skb)->protocol == IPPROTO_UDP &&
e5a498e943fbc49 Ben Hutchings   2013-12-06  1634  		skb_headlen(skb) >=
e5a498e943fbc49 Ben Hutchings   2013-12-06  1635  		skb_transport_offset(skb) + sizeof(struct udphdr) &&
7c236c43b838221 Stuart Hodgson  2012-09-03 @1636  		udp_hdr(skb)->dest == htons(PTP_EVENT_PORT);
7c236c43b838221 Stuart Hodgson  2012-09-03  1637  }
7c236c43b838221 Stuart Hodgson  2012-09-03  1638  
7c236c43b838221 Stuart Hodgson  2012-09-03  1639  /* Receive a PTP packet.  Packets are queued until the arrival of
7c236c43b838221 Stuart Hodgson  2012-09-03  1640   * the receive timestamp from the MC - this will probably occur after the
7c236c43b838221 Stuart Hodgson  2012-09-03  1641   * packet arrival because of the processing in the MC.
7c236c43b838221 Stuart Hodgson  2012-09-03  1642   */
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1643  static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
7c236c43b838221 Stuart Hodgson  2012-09-03  1644  {
7c236c43b838221 Stuart Hodgson  2012-09-03  1645  	struct efx_nic *efx = channel->efx;
7c236c43b838221 Stuart Hodgson  2012-09-03  1646  	struct efx_ptp_data *ptp = efx->ptp_data;
7c236c43b838221 Stuart Hodgson  2012-09-03  1647  	struct efx_ptp_match *match = (struct efx_ptp_match *)skb->cb;
c939a316459783e Laurence Evans  2012-11-15  1648  	u8 *match_data_012, *match_data_345;
7c236c43b838221 Stuart Hodgson  2012-09-03  1649  	unsigned int version;
ce320f44d677549 Ben Hutchings   2014-02-12  1650  	u8 *data;
7c236c43b838221 Stuart Hodgson  2012-09-03  1651  
7c236c43b838221 Stuart Hodgson  2012-09-03  1652  	match->expiry = jiffies + msecs_to_jiffies(PKT_EVENT_LIFETIME_MS);
7c236c43b838221 Stuart Hodgson  2012-09-03  1653  
7c236c43b838221 Stuart Hodgson  2012-09-03  1654  	/* Correct version? */
7c236c43b838221 Stuart Hodgson  2012-09-03  1655  	if (ptp->mode == MC_CMD_PTP_MODE_V1) {
97d48a10c670f87 Alexandre Rames 2013-01-11  1656  		if (!pskb_may_pull(skb, PTP_V1_MIN_LENGTH)) {
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1657  			return false;
7c236c43b838221 Stuart Hodgson  2012-09-03  1658  		}
ce320f44d677549 Ben Hutchings   2014-02-12  1659  		data = skb->data;
ce320f44d677549 Ben Hutchings   2014-02-12  1660  		version = ntohs(*(__be16 *)&data[PTP_V1_VERSION_OFFSET]);
7c236c43b838221 Stuart Hodgson  2012-09-03  1661  		if (version != PTP_VERSION_V1) {
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1662  			return false;
7c236c43b838221 Stuart Hodgson  2012-09-03  1663  		}
c939a316459783e Laurence Evans  2012-11-15  1664  
c939a316459783e Laurence Evans  2012-11-15  1665  		/* PTP V1 uses all six bytes of the UUID to match the packet
c939a316459783e Laurence Evans  2012-11-15  1666  		 * to the timestamp
c939a316459783e Laurence Evans  2012-11-15  1667  		 */
ce320f44d677549 Ben Hutchings   2014-02-12  1668  		match_data_012 = data + PTP_V1_UUID_OFFSET;
ce320f44d677549 Ben Hutchings   2014-02-12  1669  		match_data_345 = data + PTP_V1_UUID_OFFSET + 3;
7c236c43b838221 Stuart Hodgson  2012-09-03  1670  	} else {
97d48a10c670f87 Alexandre Rames 2013-01-11  1671  		if (!pskb_may_pull(skb, PTP_V2_MIN_LENGTH)) {
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1672  			return false;
7c236c43b838221 Stuart Hodgson  2012-09-03  1673  		}
ce320f44d677549 Ben Hutchings   2014-02-12  1674  		data = skb->data;
ce320f44d677549 Ben Hutchings   2014-02-12  1675  		version = data[PTP_V2_VERSION_OFFSET];
7c236c43b838221 Stuart Hodgson  2012-09-03  1676  		if ((version & PTP_VERSION_V2_MASK) != PTP_VERSION_V2) {
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1677  			return false;
7c236c43b838221 Stuart Hodgson  2012-09-03  1678  		}
c939a316459783e Laurence Evans  2012-11-15  1679  
c939a316459783e Laurence Evans  2012-11-15  1680  		/* The original V2 implementation uses bytes 2-7 of
c939a316459783e Laurence Evans  2012-11-15  1681  		 * the UUID to match the packet to the timestamp. This
c939a316459783e Laurence Evans  2012-11-15  1682  		 * discards two of the bytes of the MAC address used
c939a316459783e Laurence Evans  2012-11-15  1683  		 * to create the UUID (SF bug 33070).  The PTP V2
c939a316459783e Laurence Evans  2012-11-15  1684  		 * enhanced mode fixes this issue and uses bytes 0-2
c939a316459783e Laurence Evans  2012-11-15  1685  		 * and byte 5-7 of the UUID.
c939a316459783e Laurence Evans  2012-11-15  1686  		 */
ce320f44d677549 Ben Hutchings   2014-02-12  1687  		match_data_345 = data + PTP_V2_UUID_OFFSET + 5;
c939a316459783e Laurence Evans  2012-11-15  1688  		if (ptp->mode == MC_CMD_PTP_MODE_V2) {
ce320f44d677549 Ben Hutchings   2014-02-12  1689  			match_data_012 = data + PTP_V2_UUID_OFFSET + 2;
c939a316459783e Laurence Evans  2012-11-15  1690  		} else {
ce320f44d677549 Ben Hutchings   2014-02-12  1691  			match_data_012 = data + PTP_V2_UUID_OFFSET + 0;
c939a316459783e Laurence Evans  2012-11-15  1692  			BUG_ON(ptp->mode != MC_CMD_PTP_MODE_V2_ENHANCED);
c939a316459783e Laurence Evans  2012-11-15  1693  		}
7c236c43b838221 Stuart Hodgson  2012-09-03  1694  	}
7c236c43b838221 Stuart Hodgson  2012-09-03  1695  
7c236c43b838221 Stuart Hodgson  2012-09-03  1696  	/* Does this packet require timestamping? */
ce320f44d677549 Ben Hutchings   2014-02-12 @1697  	if (ntohs(*(__be16 *)&data[PTP_DPORT_OFFSET]) == PTP_EVENT_PORT) {
7c236c43b838221 Stuart Hodgson  2012-09-03  1698  		match->state = PTP_PACKET_STATE_UNMATCHED;
7c236c43b838221 Stuart Hodgson  2012-09-03  1699  
c939a316459783e Laurence Evans  2012-11-15  1700  		/* We expect the sequence number to be in the same position in
c939a316459783e Laurence Evans  2012-11-15  1701  		 * the packet for PTP V1 and V2
c939a316459783e Laurence Evans  2012-11-15  1702  		 */
c939a316459783e Laurence Evans  2012-11-15  1703  		BUILD_BUG_ON(PTP_V1_SEQUENCE_OFFSET != PTP_V2_SEQUENCE_OFFSET);
c939a316459783e Laurence Evans  2012-11-15  1704  		BUILD_BUG_ON(PTP_V1_SEQUENCE_LENGTH != PTP_V2_SEQUENCE_LENGTH);
c939a316459783e Laurence Evans  2012-11-15  1705  
7c236c43b838221 Stuart Hodgson  2012-09-03  1706  		/* Extract UUID/Sequence information */
c939a316459783e Laurence Evans  2012-11-15  1707  		match->words[0] = (match_data_012[0]         |
c939a316459783e Laurence Evans  2012-11-15  1708  				   (match_data_012[1] << 8)  |
c939a316459783e Laurence Evans  2012-11-15  1709  				   (match_data_012[2] << 16) |
c939a316459783e Laurence Evans  2012-11-15  1710  				   (match_data_345[0] << 24));
c939a316459783e Laurence Evans  2012-11-15  1711  		match->words[1] = (match_data_345[1]         |
c939a316459783e Laurence Evans  2012-11-15  1712  				   (match_data_345[2] << 8)  |
ce320f44d677549 Ben Hutchings   2014-02-12  1713  				   (data[PTP_V1_SEQUENCE_OFFSET +
7c236c43b838221 Stuart Hodgson  2012-09-03  1714  					 PTP_V1_SEQUENCE_LENGTH - 1] <<
7c236c43b838221 Stuart Hodgson  2012-09-03  1715  				    16));
7c236c43b838221 Stuart Hodgson  2012-09-03  1716  	} else {
7c236c43b838221 Stuart Hodgson  2012-09-03  1717  		match->state = PTP_PACKET_STATE_MATCH_UNWANTED;
7c236c43b838221 Stuart Hodgson  2012-09-03  1718  	}
7c236c43b838221 Stuart Hodgson  2012-09-03  1719  
7c236c43b838221 Stuart Hodgson  2012-09-03  1720  	skb_queue_tail(&ptp->rxq, skb);
7c236c43b838221 Stuart Hodgson  2012-09-03  1721  	queue_work(ptp->workwq, &ptp->work);
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1722  
4a74dc65e3ad825 Ben Hutchings   2013-03-05  1723  	return true;
7c236c43b838221 Stuart Hodgson  2012-09-03  1724  }
7c236c43b838221 Stuart Hodgson  2012-09-03  1725  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH net-next v2 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
PTP") added support for hardware timestamping on TX for cards of the
8000 series and newer, in an effort to provide support for other
transports other than IPv4/UDP.

However, timestamping was still not working on RX for these other
transports. This patch add support for PTP over IPv6/UDP.

Tested: sync as master and as slave is correct using ptp4l from linuxptp
package, both with IPv4 and IPv6.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/filter.h | 22 +++++++++++
 drivers/net/ethernet/sfc/ptp.c    | 61 ++++++++++++++++++++++++-------
 2 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/sfc/filter.h b/drivers/net/ethernet/sfc/filter.h
index 4d928839d292..be72e71da027 100644
--- a/drivers/net/ethernet/sfc/filter.h
+++ b/drivers/net/ethernet/sfc/filter.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/in6.h>
 #include <asm/byteorder.h>
 
 /**
@@ -223,6 +224,27 @@ efx_filter_set_ipv4_local(struct efx_filter_spec *spec, u8 proto,
 	return 0;
 }
 
+/**
+ * efx_filter_set_ipv6_local - specify IPv6 host, transport protocol and port
+ * @spec: Specification to initialise
+ * @proto: Transport layer protocol number
+ * @host: Local host address (network byte order)
+ * @port: Local port (network byte order)
+ */
+static inline int
+efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
+			  const struct in6_addr *host, __be16 port)
+{
+	spec->match_flags |=
+		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
+		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT;
+	spec->ether_type = htons(ETH_P_IPV6);
+	spec->ip_proto = proto;
+	memcpy(spec->loc_host, host, sizeof(spec->loc_host));
+	spec->loc_port = port;
+	return 0;
+}
+
 /**
  * efx_filter_set_ipv4_full - specify IPv4 hosts, transport protocol and ports
  * @spec: Specification to initialise
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index bdf6c09cfac5..14497f8c762c 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	2
+#define PTP_RXFILTERS_LEN	4
 
-#define PTP_ADDRESS		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_ADDR_IPV4		htonl(0xe0000181)	/* 224.0.1.129 */
+#define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+				0, 0x01, 0x81}		/* ff0e::181 */
 #define PTP_EVENT_PORT		htons(319)
 #define PTP_GENERAL_PORT	htons(320)
 
@@ -1297,28 +1299,48 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 	}
 }
 
-static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
+static inline void efx_ptp_init_filter(struct efx_nic *efx,
+				       struct efx_filter_spec *rxfilter)
 {
-	struct efx_ptp_data *ptp = efx->ptp_data;
-	struct efx_filter_spec rxfilter;
-	int rc;
+	struct efx_channel *channel = efx->ptp_data->channel;
+	struct efx_rx_queue *queue = efx_channel_get_rx_queue(channel);
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
+	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
+			   efx_rx_queue_index(queue));
+}
 
-	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDRESS, port);
+static inline int efx_ptp_insert_filter(struct efx_nic *efx,
+					struct efx_filter_spec *rxfilter)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	int rc = efx_filter_insert_filter(efx, rxfilter, true);
 	if (rc < 0)
 		return rc;
-
 	ptp->rxfilters[ptp->rxfilters_count] = rc;
 	ptp->rxfilters_count++;
-
 	return 0;
 }
 
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, __be16 port)
+{
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, PTP_ADDR_IPV4, port);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
+static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, __be16 port)
+{
+	const struct in6_addr addr = {{PTP_ADDR_IPV6}};
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv6_local(&rxfilter, IPPROTO_UDP, &addr, port);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1335,6 +1357,19 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 	if (rc < 0)
 		goto fail;
 
+	/* if the NIC supports hw timestamps by the MAC, we can support
+	 * PTP over IPv6
+	 */
+	if (efx_ptp_use_mac_tx_timestamps(efx)) {
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
+		if (rc < 0)
+			goto fail;
+
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
+		if (rc < 0)
+			goto fail;
+	}
+
 	return 0;
 
 fail:
-- 
2.34.1

[PATCH net-next v2 3/3] sfc: support PTP over Ethernet

[Íñigo Huguet]

The previous patch add support for PTP over IPv6/UDP (only for 8000
series and newer) and this one add support for PTP over 802.3.

Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
and IPv6 still works fine.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 14497f8c762c..c6218dceddaa 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,13 +118,14 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	4
+#define PTP_RXFILTERS_LEN	5
 
 #define PTP_ADDR_IPV4		htonl(0xe0000181)	/* 224.0.1.129 */
 #define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 				0, 0x01, 0x81}		/* ff0e::181 */
 #define PTP_EVENT_PORT		htons(319)
 #define PTP_GENERAL_PORT	htons(320)
+#define PTP_ADDR_ETHER		{0x01, 0x1b, 0x19, 0, 0, 0} /* 01-1B-19-00-00-00 */
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -1341,6 +1342,18 @@ static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, __be16 port)
 	return efx_ptp_insert_filter(efx, &rxfilter);
 }
 
+static int efx_ptp_insert_eth_filter(struct efx_nic *efx)
+{
+	const u8 addr[ETH_ALEN] = PTP_ADDR_ETHER;
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_eth_local(&rxfilter, EFX_FILTER_VID_UNSPEC, addr);
+	rxfilter.match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+	rxfilter.ether_type = htons(ETH_P_1588);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1358,7 +1371,7 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		goto fail;
 
 	/* if the NIC supports hw timestamps by the MAC, we can support
-	 * PTP over IPv6
+	 * PTP over IPv6 and Ethernet
 	 */
 	if (efx_ptp_use_mac_tx_timestamps(efx)) {
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
@@ -1368,6 +1381,10 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
 		if (rc < 0)
 			goto fail;
+
+		rc = efx_ptp_insert_eth_filter(efx);
+		if (rc < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.34.1

[PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

Most recent cards (8000 series and newer) had enough hardware support
for this, but it was not enabled in the driver. The transmission of PTP
packets over these protocols was already added in commit bd4a2697e5e2
("sfc: use hardware tx timestamps for more than PTP"), but receiving
them was already unsupported so synchronization didn't happen.

These patches add support for timestamping received packets over
IPv6/UPD and IEEE802.3.

v2: fixed weird indentation in efx_ptp_init_filter
v3: fixed bug caused by usage of htons in PTP_EVENT_PORT definition.
    It was used in more places, where htons was used too, so using it
    2 times leave it again in host order. I didn't detected it in my
    tests because it only affected if timestamping through the MC, but
    the model I used do it through the MAC. Detected by kernel test
    robot <lkp@intel.com>

Íñigo Huguet (3):
  sfc: allow more flexible way of adding filters for PTP
  sfc: support PTP over IPv6/UDP
  sfc: support PTP over Ethernet

 drivers/net/ethernet/sfc/filter.h |  22 +++++
 drivers/net/ethernet/sfc/ptp.c    | 131 ++++++++++++++++++++----------
 2 files changed, 111 insertions(+), 42 deletions(-)

-- 
2.34.1

[PATCH net-next v3 1/3] sfc: allow more flexible way of adding filters for PTP

[Íñigo Huguet]

In preparation for the support of PTP over IPv6/UDP and Ethernet in next
patches, allow a more flexible way of adding and removing RX filters for
PTP. Right now, only 2 filters are allowed, which are the ones needed
for PTP over IPv4/UDP.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 67 ++++++++++++++++------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 10ad0b93d283..719005d79943 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,6 +118,8 @@
 
 #define	PTP_MIN_LENGTH		63
 
+#define PTP_RXFILTERS_LEN	2
+
 #define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
@@ -224,9 +226,8 @@ struct efx_ptp_timeset {
  * @work: Work task
  * @reset_required: A serious error has occurred and the PTP task needs to be
  *                  reset (disable, enable).
- * @rxfilter_event: Receive filter when operating
- * @rxfilter_general: Receive filter when operating
- * @rxfilter_installed: Receive filter installed
+ * @rxfilters: Receive filters when operating
+ * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
  * @config: Current timestamp configuration
  * @enabled: PTP operation enabled
  * @mode: Mode in which PTP operating (PTP version)
@@ -295,9 +296,8 @@ struct efx_ptp_data {
 	struct workqueue_struct *workwq;
 	struct work_struct work;
 	bool reset_required;
-	u32 rxfilter_event;
-	u32 rxfilter_general;
-	bool rxfilter_installed;
+	u32 rxfilters[PTP_RXFILTERS_LEN];
+	size_t rxfilters_count;
 	struct hwtstamp_config config;
 	bool enabled;
 	unsigned int mode;
@@ -1290,61 +1290,56 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	if (ptp->rxfilter_installed) {
-		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_general);
+	while (ptp->rxfilters_count) {
+		ptp->rxfilters_count--;
 		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_event);
-		ptp->rxfilter_installed = false;
+					  ptp->rxfilters[ptp->rxfilters_count]);
 	}
 }
 
-static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 	struct efx_filter_spec rxfilter;
 	int rc;
 
-	if (!ptp->channel || ptp->rxfilter_installed)
-		return 0;
-
-	/* Must filter on both event and general ports to ensure
-	 * that there is no packet re-ordering.
-	 */
 	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
 			   efx_rx_queue_index(
 				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_EVENT_PORT));
-	if (rc != 0)
-		return rc;
+
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
+				  htons(port));
 
 	rc = efx_filter_insert_filter(efx, &rxfilter, true);
 	if (rc < 0)
 		return rc;
-	ptp->rxfilter_event = rc;
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_GENERAL_PORT));
-	if (rc != 0)
+	ptp->rxfilters[ptp->rxfilters_count] = rc;
+	ptp->rxfilters_count++;
+
+	return 0;
+}
+
+static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
+	int rc;
+
+	if (!ptp->channel || ptp->rxfilters_count)
+		return 0;
+
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
+	if (rc < 0)
 		goto fail;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
 	if (rc < 0)
 		goto fail;
-	ptp->rxfilter_general = rc;
 
-	ptp->rxfilter_installed = true;
 	return 0;
 
 fail:
-	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-				  ptp->rxfilter_event);
+	efx_ptp_remove_multicast_filters(efx);
 	return rc;
 }
 
-- 
2.34.1

[PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
PTP") added support for hardware timestamping on TX for cards of the
8000 series and newer, in an effort to provide support for other
transports other than IPv4/UDP.

However, timestamping was still not working on RX for these other
transports. This patch add support for PTP over IPv6/UDP.

Tested: sync as master and as slave is correct using ptp4l from linuxptp
package, both with IPv4 and IPv6.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/filter.h | 22 +++++++++++
 drivers/net/ethernet/sfc/ptp.c    | 63 ++++++++++++++++++++++++-------
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/sfc/filter.h b/drivers/net/ethernet/sfc/filter.h
index 4d928839d292..be72e71da027 100644
--- a/drivers/net/ethernet/sfc/filter.h
+++ b/drivers/net/ethernet/sfc/filter.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/in6.h>
 #include <asm/byteorder.h>
 
 /**
@@ -223,6 +224,27 @@ efx_filter_set_ipv4_local(struct efx_filter_spec *spec, u8 proto,
 	return 0;
 }
 
+/**
+ * efx_filter_set_ipv6_local - specify IPv6 host, transport protocol and port
+ * @spec: Specification to initialise
+ * @proto: Transport layer protocol number
+ * @host: Local host address (network byte order)
+ * @port: Local port (network byte order)
+ */
+static inline int
+efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
+			  const struct in6_addr *host, __be16 port)
+{
+	spec->match_flags |=
+		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
+		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT;
+	spec->ether_type = htons(ETH_P_IPV6);
+	spec->ip_proto = proto;
+	memcpy(spec->loc_host, host, sizeof(spec->loc_host));
+	spec->loc_port = port;
+	return 0;
+}
+
 /**
  * efx_filter_set_ipv4_full - specify IPv4 hosts, transport protocol and ports
  * @spec: Specification to initialise
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 719005d79943..060525ac2baf 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	2
+#define PTP_RXFILTERS_LEN	4
 
-#define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
 
@@ -1297,29 +1299,49 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 	}
 }
 
-static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+static inline void efx_ptp_init_filter(struct efx_nic *efx,
+				       struct efx_filter_spec *rxfilter)
 {
-	struct efx_ptp_data *ptp = efx->ptp_data;
-	struct efx_filter_spec rxfilter;
-	int rc;
+	struct efx_channel *channel = efx->ptp_data->channel;
+	struct efx_rx_queue *queue = efx_channel_get_rx_queue(channel);
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
+	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
+			   efx_rx_queue_index(queue));
+}
 
-	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
-				  htons(port));
+static inline int efx_ptp_insert_filter(struct efx_nic *efx,
+					struct efx_filter_spec *rxfilter)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	int rc = efx_filter_insert_filter(efx, rxfilter, true);
 	if (rc < 0)
 		return rc;
-
 	ptp->rxfilters[ptp->rxfilters_count] = rc;
 	ptp->rxfilters_count++;
-
 	return 0;
 }
 
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+{
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDR_IPV4),
+				  htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
+static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
+{
+	const struct in6_addr addr = {{PTP_ADDR_IPV6}};
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv6_local(&rxfilter, IPPROTO_UDP, &addr, htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1336,6 +1358,19 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 	if (rc < 0)
 		goto fail;
 
+	/* if the NIC supports hw timestamps by the MAC, we can support
+	 * PTP over IPv6
+	 */
+	if (efx_ptp_use_mac_tx_timestamps(efx)) {
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
+		if (rc < 0)
+			goto fail;
+
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
+		if (rc < 0)
+			goto fail;
+	}
+
 	return 0;
 
 fail:
-- 
2.34.1

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Jakub Kicinski]

On Thu, 25 Aug 2022 11:02:41 +0200 Íñigo Huguet wrote:
> -static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
> +static inline void efx_ptp_init_filter(struct efx_nic *efx,
> +				       struct efx_filter_spec *rxfilter)

No static inline in sources unless you actually checked and the
compiler does something stupid (pls mention it in the commit message 
in that case).

> +static inline int
> +efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
> +			  const struct in6_addr *host, __be16 port)

also - unclear why this is defined in the header

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

On Fri, Aug 26, 2022 at 3:32 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 25 Aug 2022 11:02:41 +0200 Íñigo Huguet wrote:
> > -static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
> > +static inline void efx_ptp_init_filter(struct efx_nic *efx,
> > +                                    struct efx_filter_spec *rxfilter)
>
> No static inline in sources unless you actually checked and the
> compiler does something stupid (pls mention it in the commit message
> in that case).

OK, I will change it (I think I should read again and remember the
coding style document)

>
> > +static inline int
> > +efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
> > +                       const struct in6_addr *host, __be16 port)
>
> also - unclear why this is defined in the header
>

This is just because it's the equivalent of other already existing
similar functions in that file. I think I should keep this one
untouched for cohesion.
-- 
Íñigo Huguet

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Jakub Kicinski]

On Fri, 26 Aug 2022 08:39:44 +0200 Íñigo Huguet wrote:
> > > +static inline int
> > > +efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
> > > +                       const struct in6_addr *host, __be16 port)  
> >
> > also - unclear why this is defined in the header
> 
> This is just because it's the equivalent of other already existing
> similar functions in that file. I think I should keep this one
> untouched for cohesion.

We usually defer refactoring for coding style issues until someone 
is otherwise touching the code, so surrounding code doing something
against the guidance may be misleading.

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

On Sat, Aug 27, 2022 at 1:27 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 26 Aug 2022 08:39:44 +0200 Íñigo Huguet wrote:
> > > > +static inline int
> > > > +efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
> > > > +                       const struct in6_addr *host, __be16 port)
> > >
> > > also - unclear why this is defined in the header
> >
> > This is just because it's the equivalent of other already existing
> > similar functions in that file. I think I should keep this one
> > untouched for cohesion.
>
> We usually defer refactoring for coding style issues until someone
> is otherwise touching the code, so surrounding code doing something
> against the guidance may be misleading.
>

Yes but I'm not sure what I should do in this case... all other
efx_filter_xxx functions are in filter.h, so putting this one in a
different place could make it difficult to understand how the files
are organized. Should I put the declaration in the header (without
`inline`) and the definition in a new filter.c file? Should I move all
other definitions to this new file?

Also, what's exactly the rule, apart from not using `inline`, to avoid
doing the same thing again: to avoid function definitions directly in
header files?

Thanks
-- 
Íñigo Huguet

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Jakub Kicinski]

On Mon, 29 Aug 2022 09:03:44 +0200 Íñigo Huguet wrote:
> > We usually defer refactoring for coding style issues until someone
> > is otherwise touching the code, so surrounding code doing something
> > against the guidance may be misleading.
> 
> Yes but I'm not sure what I should do in this case... all other
> efx_filter_xxx functions are in filter.h, so putting this one in a
> different place could make it difficult to understand how the files
> are organized. Should I put the declaration in the header (without
> `inline`) and the definition in a new filter.c file? Should I move all
> other definitions to this new file?

Hm, I see, perhaps adding a new filter.c would be too much for your set.
Let's leave the definition in the header then.

> Also, what's exactly the rule, apart from not using `inline`, to avoid
> doing the same thing again: to avoid function definitions directly in
> header files?

Not sure I'm parsing the question right, but it's okay to add small
functions in local headers. Here it seem to have only been used in
one place, and I didn't see the context.

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

On Tue, Aug 30, 2022 at 2:29 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 29 Aug 2022 09:03:44 +0200 Íñigo Huguet wrote:
> > > We usually defer refactoring for coding style issues until someone
> > > is otherwise touching the code, so surrounding code doing something
> > > against the guidance may be misleading.
> >
> > Yes but I'm not sure what I should do in this case... all other
> > efx_filter_xxx functions are in filter.h, so putting this one in a
> > different place could make it difficult to understand how the files
> > are organized. Should I put the declaration in the header (without
> > `inline`) and the definition in a new filter.c file? Should I move all
> > other definitions to this new file?
>
> Hm, I see, perhaps adding a new filter.c would be too much for your set.
> Let's leave the definition in the header then.
>
> > Also, what's exactly the rule, apart from not using `inline`, to avoid
> > doing the same thing again: to avoid function definitions directly in
> > header files?
>
> Not sure I'm parsing the question right, but it's okay to add small
> functions in local headers. Here it seem to have only been used in
> one place, and I didn't see the context.
>

I expresed it terribly badly, but you parsed it right. Thanks, now I
understand what your concern was.

-- 
Íñigo Huguet

Re: [PATCH net-next v3 2/3] sfc: support PTP over IPv6/UDP

[Edward Cree]

On 26/08/2022 07:39, Íñigo Huguet wrote:
> On Fri, Aug 26, 2022 at 3:32 AM Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> On Thu, 25 Aug 2022 11:02:41 +0200 Íñigo Huguet wrote:
>>> +static inline int
>>> +efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
>>> +                       const struct in6_addr *host, __be16 port)
>>
>> also - unclear why this is defined in the header
>>
> 
> This is just because it's the equivalent of other already existing
> similar functions in that file. I think I should keep this one
> untouched for cohesion.
My preference would be to keep this in filter.h as Íñigo currently
 has it, to follow the existing pattern.  These "populate a filter
 spec" functions are really just typesafe macros.

-ed

[PATCH net-next v3 3/3] sfc: support PTP over Ethernet

[Íñigo Huguet]

The previous patch add support for PTP over IPv6/UDP (only for 8000
series and newer) and this one add support for PTP over 802.3.

Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
and IPv6 still works fine.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 060525ac2baf..9547b2e4f7f3 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,13 +118,14 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	4
+#define PTP_RXFILTERS_LEN	5
 
 #define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
 #define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
+#define PTP_ADDR_ETHER		{0x01, 0x1b, 0x19, 0, 0, 0} /* 01-1B-19-00-00-00 */
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -1342,6 +1343,18 @@ static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
 	return efx_ptp_insert_filter(efx, &rxfilter);
 }
 
+static int efx_ptp_insert_eth_filter(struct efx_nic *efx)
+{
+	const u8 addr[ETH_ALEN] = PTP_ADDR_ETHER;
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_eth_local(&rxfilter, EFX_FILTER_VID_UNSPEC, addr);
+	rxfilter.match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+	rxfilter.ether_type = htons(ETH_P_1588);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1359,7 +1372,7 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		goto fail;
 
 	/* if the NIC supports hw timestamps by the MAC, we can support
-	 * PTP over IPv6
+	 * PTP over IPv6 and Ethernet
 	 */
 	if (efx_ptp_use_mac_tx_timestamps(efx)) {
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
@@ -1369,6 +1382,10 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
 		if (rc < 0)
 			goto fail;
+
+		rc = efx_ptp_insert_eth_filter(efx);
+		if (rc < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Andrew Lunn]

On Thu, Aug 25, 2022 at 11:02:39AM +0200, Íñigo Huguet wrote:
> Most recent cards (8000 series and newer) had enough hardware support
> for this, but it was not enabled in the driver. The transmission of PTP
> packets over these protocols was already added in commit bd4a2697e5e2
> ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> them was already unsupported so synchronization didn't happen.

You don't appear to Cc: the PTP maintainer.

    Andrew

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

On Thu, Aug 25, 2022 at 6:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Aug 25, 2022 at 11:02:39AM +0200, Íñigo Huguet wrote:
> > Most recent cards (8000 series and newer) had enough hardware support
> > for this, but it was not enabled in the driver. The transmission of PTP
> > packets over these protocols was already added in commit bd4a2697e5e2
> > ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> > them was already unsupported so synchronization didn't happen.
>
> You don't appear to Cc: the PTP maintainer.
>
>     Andrew
>

I didn't think about that, but looking at MAINTAINERS, there doesn't
seem to be any. There are 2 maintainers for the drivers of the clock
devices, but none for anything related to the network protocol...

-- 
Íñigo Huguet

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Andrew Lunn]

On Fri, Aug 26, 2022 at 08:58:31AM +0200, Íñigo Huguet wrote:
> On Thu, Aug 25, 2022 at 6:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Thu, Aug 25, 2022 at 11:02:39AM +0200, Íñigo Huguet wrote:
> > > Most recent cards (8000 series and newer) had enough hardware support
> > > for this, but it was not enabled in the driver. The transmission of PTP
> > > packets over these protocols was already added in commit bd4a2697e5e2
> > > ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> > > them was already unsupported so synchronization didn't happen.
> >
> > You don't appear to Cc: the PTP maintainer.
> >
> >     Andrew
> >
> 
> I didn't think about that, but looking at MAINTAINERS, there doesn't
> seem to be any. There are 2 maintainers for the drivers of the clock
> devices, but none for anything related to the network protocol...

PTP HARDWARE CLOCK SUPPORT
M:      Richard Cochran <richardcochran@gmail.com>
L:      netdev@vger.kernel.org
S:      Maintained
W:      http://linuxptp.sourceforge.net/
F:      Documentation/ABI/testing/sysfs-ptp
F:      Documentation/driver-api/ptp.rst
F:      drivers/net/phy/dp83640*
F:      drivers/ptp/*
F:      include/linux/ptp_cl*

I assume you are using linuxptp with this?

  Andrew

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

On Fri, Aug 26, 2022 at 2:52 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Aug 26, 2022 at 08:58:31AM +0200, Íñigo Huguet wrote:
> > On Thu, Aug 25, 2022 at 6:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Thu, Aug 25, 2022 at 11:02:39AM +0200, Íñigo Huguet wrote:
> > > > Most recent cards (8000 series and newer) had enough hardware support
> > > > for this, but it was not enabled in the driver. The transmission of PTP
> > > > packets over these protocols was already added in commit bd4a2697e5e2
> > > > ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> > > > them was already unsupported so synchronization didn't happen.
> > >
> > > You don't appear to Cc: the PTP maintainer.
> > >
> > >     Andrew
> > >
> >
> > I didn't think about that, but looking at MAINTAINERS, there doesn't
> > seem to be any. There are 2 maintainers for the drivers of the clock
> > devices, but none for anything related to the network protocol...
>
> PTP HARDWARE CLOCK SUPPORT
> M:      Richard Cochran <richardcochran@gmail.com>
> L:      netdev@vger.kernel.org
> S:      Maintained
> W:      http://linuxptp.sourceforge.net/
> F:      Documentation/ABI/testing/sysfs-ptp
> F:      Documentation/driver-api/ptp.rst
> F:      drivers/net/phy/dp83640*
> F:      drivers/ptp/*
> F:      include/linux/ptp_cl*
>
> I assume you are using linuxptp with this?
>
>   Andrew
>

I'm still not sure if this falls under his responsibility because it
doesn't affect the part of the clock device, but in any case it
doesn't hurt to CC him as you suggest. Thanks!

Richard, missed to CC you in this patch series, just in case it's of
your interest.

-- 
Íñigo Huguet

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Richard Cochran]

On Mon, Aug 29, 2022 at 09:09:48AM +0200, Íñigo Huguet wrote:
> Richard, missed to CC you in this patch series, just in case it's of
> your interest.

I do appreciate being on CC for anything PTP related.

Thanks,

Richard

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Andrew Lunn]

On Mon, Aug 29, 2022 at 07:38:36AM -0700, Richard Cochran wrote:
> On Mon, Aug 29, 2022 at 09:09:48AM +0200, Íñigo Huguet wrote:
> > Richard, missed to CC you in this patch series, just in case it's of
> > your interest.
> 
> I do appreciate being on CC for anything PTP related.

Russell King had the issue he was being missed on a lot of PHYLINK
patches. He updated the MAINTAINERS entry with:

K:	phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)

Maybe you could add some sort of regex for common ptp functions and structures?

      Andrew

Re: [PATCH net-next v3 0/3] sfc: add support for PTP over IPv6 and 802.3

[Jakub Kicinski]

On Mon, 29 Aug 2022 18:15:16 +0200 Andrew Lunn wrote:
> On Mon, Aug 29, 2022 at 07:38:36AM -0700, Richard Cochran wrote:
> > On Mon, Aug 29, 2022 at 09:09:48AM +0200, Íñigo Huguet wrote:  
> > > Richard, missed to CC you in this patch series, just in case it's of
> > > your interest.  
> > 
> > I do appreciate being on CC for anything PTP related.  
> 
> Russell King had the issue he was being missed on a lot of PHYLINK
> patches. He updated the MAINTAINERS entry with:
> 
> K:	phylink\.h|struct\s+phylink|\.phylink|>phylink_|phylink_(autoneg|clear|connect|create|destroy|disconnect|ethtool|helper|mac|mii|of|set|start|stop|test|validate)
> 
> Maybe you could add some sort of regex for common ptp functions and structures?

+1

[PATCH net-next v4 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

Most recent cards (8000 series and newer) had enough hardware support
for this, but it was not enabled in the driver. The transmission of PTP
packets over these protocols was already added in commit bd4a2697e5e2
("sfc: use hardware tx timestamps for more than PTP"), but receiving
them was already unsupported so synchronization didn't happen.

These patches add support for timestamping received packets over
IPv6/UPD and IEEE802.3.

v2: fixed weird indentation in efx_ptp_init_filter
v3: fixed bug caused by usage of htons in PTP_EVENT_PORT definition.
    It was used in more places, where htons was used too, so using it
    2 times leave it again in host order. I didn't detected it in my
    tests because it only affected if timestamping through the MC, but
    the model I used do it through the MAC. Detected by kernel test
    robot <lkp@intel.com>
v4: removed `inline` specifiers from 2 local functions

Íñigo Huguet (3):
  sfc: allow more flexible way of adding filters for PTP
  sfc: support PTP over IPv6/UDP
  sfc: support PTP over Ethernet

 drivers/net/ethernet/sfc/filter.h |  22 +++++
 drivers/net/ethernet/sfc/ptp.c    | 131 ++++++++++++++++++++----------
 2 files changed, 111 insertions(+), 42 deletions(-)

-- 
2.34.1

[PATCH net-next v4 1/3] sfc: allow more flexible way of adding filters for PTP

[Íñigo Huguet]

In preparation for the support of PTP over IPv6/UDP and Ethernet in next
patches, allow a more flexible way of adding and removing RX filters for
PTP. Right now, only 2 filters are allowed, which are the ones needed
for PTP over IPv4/UDP.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 67 ++++++++++++++++------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 10ad0b93d283..719005d79943 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,6 +118,8 @@
 
 #define	PTP_MIN_LENGTH		63
 
+#define PTP_RXFILTERS_LEN	2
+
 #define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
@@ -224,9 +226,8 @@ struct efx_ptp_timeset {
  * @work: Work task
  * @reset_required: A serious error has occurred and the PTP task needs to be
  *                  reset (disable, enable).
- * @rxfilter_event: Receive filter when operating
- * @rxfilter_general: Receive filter when operating
- * @rxfilter_installed: Receive filter installed
+ * @rxfilters: Receive filters when operating
+ * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
  * @config: Current timestamp configuration
  * @enabled: PTP operation enabled
  * @mode: Mode in which PTP operating (PTP version)
@@ -295,9 +296,8 @@ struct efx_ptp_data {
 	struct workqueue_struct *workwq;
 	struct work_struct work;
 	bool reset_required;
-	u32 rxfilter_event;
-	u32 rxfilter_general;
-	bool rxfilter_installed;
+	u32 rxfilters[PTP_RXFILTERS_LEN];
+	size_t rxfilters_count;
 	struct hwtstamp_config config;
 	bool enabled;
 	unsigned int mode;
@@ -1290,61 +1290,56 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	if (ptp->rxfilter_installed) {
-		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_general);
+	while (ptp->rxfilters_count) {
+		ptp->rxfilters_count--;
 		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_event);
-		ptp->rxfilter_installed = false;
+					  ptp->rxfilters[ptp->rxfilters_count]);
 	}
 }
 
-static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 	struct efx_filter_spec rxfilter;
 	int rc;
 
-	if (!ptp->channel || ptp->rxfilter_installed)
-		return 0;
-
-	/* Must filter on both event and general ports to ensure
-	 * that there is no packet re-ordering.
-	 */
 	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
 			   efx_rx_queue_index(
 				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_EVENT_PORT));
-	if (rc != 0)
-		return rc;
+
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
+				  htons(port));
 
 	rc = efx_filter_insert_filter(efx, &rxfilter, true);
 	if (rc < 0)
 		return rc;
-	ptp->rxfilter_event = rc;
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_GENERAL_PORT));
-	if (rc != 0)
+	ptp->rxfilters[ptp->rxfilters_count] = rc;
+	ptp->rxfilters_count++;
+
+	return 0;
+}
+
+static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
+	int rc;
+
+	if (!ptp->channel || ptp->rxfilters_count)
+		return 0;
+
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
+	if (rc < 0)
 		goto fail;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
 	if (rc < 0)
 		goto fail;
-	ptp->rxfilter_general = rc;
 
-	ptp->rxfilter_installed = true;
 	return 0;
 
 fail:
-	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-				  ptp->rxfilter_event);
+	efx_ptp_remove_multicast_filters(efx);
 	return rc;
 }
 
-- 
2.34.1

Re: [PATCH net-next v4 1/3] sfc: allow more flexible way of adding filters for PTP

[Edward Cree]

On 31/08/2022 11:16, Íñigo Huguet wrote:
> In preparation for the support of PTP over IPv6/UDP and Ethernet in next
> patches, allow a more flexible way of adding and removing RX filters for
> PTP. Right now, only 2 filters are allowed, which are the ones needed
> for PTP over IPv4/UDP.
> 
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> ---
>  drivers/net/ethernet/sfc/ptp.c | 67 ++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
> index 10ad0b93d283..719005d79943 100644
> --- a/drivers/net/ethernet/sfc/ptp.c
> +++ b/drivers/net/ethernet/sfc/ptp.c
> @@ -118,6 +118,8 @@
>  
>  #define	PTP_MIN_LENGTH		63
>  
> +#define PTP_RXFILTERS_LEN	2
> +
>  #define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
>  #define PTP_EVENT_PORT		319
>  #define PTP_GENERAL_PORT	320
> @@ -224,9 +226,8 @@ struct efx_ptp_timeset {
>   * @work: Work task
>   * @reset_required: A serious error has occurred and the PTP task needs to be
>   *                  reset (disable, enable).
> - * @rxfilter_event: Receive filter when operating
> - * @rxfilter_general: Receive filter when operating
> - * @rxfilter_installed: Receive filter installed
> + * @rxfilters: Receive filters when operating
> + * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
>   * @config: Current timestamp configuration
>   * @enabled: PTP operation enabled
>   * @mode: Mode in which PTP operating (PTP version)
> @@ -295,9 +296,8 @@ struct efx_ptp_data {
>  	struct workqueue_struct *workwq;
>  	struct work_struct work;
>  	bool reset_required;
> -	u32 rxfilter_event;
> -	u32 rxfilter_general;
> -	bool rxfilter_installed;
> +	u32 rxfilters[PTP_RXFILTERS_LEN];
> +	size_t rxfilters_count;
>  	struct hwtstamp_config config;
>  	bool enabled;
>  	unsigned int mode;
> @@ -1290,61 +1290,56 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
>  {
>  	struct efx_ptp_data *ptp = efx->ptp_data;
>  
> -	if (ptp->rxfilter_installed) {
> -		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
> -					  ptp->rxfilter_general);
> +	while (ptp->rxfilters_count) {
> +		ptp->rxfilters_count--;
>  		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
> -					  ptp->rxfilter_event);
> -		ptp->rxfilter_installed = false;
> +					  ptp->rxfilters[ptp->rxfilters_count]);
>  	}
>  }
>  
> -static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
> +static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
>  {
>  	struct efx_ptp_data *ptp = efx->ptp_data;
>  	struct efx_filter_spec rxfilter;
>  	int rc;
>  
> -	if (!ptp->channel || ptp->rxfilter_installed)
> -		return 0;
> -
> -	/* Must filter on both event and general ports to ensure
> -	 * that there is no packet re-ordering.
> -	 */

Rather than losing this comment...

>  	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
>  			   efx_rx_queue_index(
>  				   efx_channel_get_rx_queue(ptp->channel)));
> -	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
> -				       htonl(PTP_ADDRESS),
> -				       htons(PTP_EVENT_PORT));
> -	if (rc != 0)
> -		return rc;
> +
> +	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
> +				  htons(port));
>  
>  	rc = efx_filter_insert_filter(efx, &rxfilter, true);
>  	if (rc < 0)
>  		return rc;
> -	ptp->rxfilter_event = rc;
>  
> -	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
> -			   efx_rx_queue_index(
> -				   efx_channel_get_rx_queue(ptp->channel)));
> -	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
> -				       htonl(PTP_ADDRESS),
> -				       htons(PTP_GENERAL_PORT));
> -	if (rc != 0)
> +	ptp->rxfilters[ptp->rxfilters_count] = rc;
> +	ptp->rxfilters_count++;
> +
> +	return 0;
> +}
> +
> +static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
> +{
> +	struct efx_ptp_data *ptp = efx->ptp_data;
> +	int rc;
> +
> +	if (!ptp->channel || ptp->rxfilters_count)
> +		return 0;
> +

... could it not migrate to here?
Otherwise looks good.
-ed

> +	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
> +	if (rc < 0)
>  		goto fail;
>  
> -	rc = efx_filter_insert_filter(efx, &rxfilter, true);
> +	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
>  	if (rc < 0)
>  		goto fail;
> -	ptp->rxfilter_general = rc;
>  
> -	ptp->rxfilter_installed = true;
>  	return 0;
>  
>  fail:
> -	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
> -				  ptp->rxfilter_event);
> +	efx_ptp_remove_multicast_filters(efx);
>  	return rc;
>  }
>  
> 

[PATCH net-next v4 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
PTP") added support for hardware timestamping on TX for cards of the
8000 series and newer, in an effort to provide support for other
transports other than IPv4/UDP.

However, timestamping was still not working on RX for these other
transports. This patch add support for PTP over IPv6/UDP.

Tested: sync as master and as slave is correct using ptp4l from linuxptp
package, both with IPv4 and IPv6.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/filter.h | 22 +++++++++++
 drivers/net/ethernet/sfc/ptp.c    | 63 ++++++++++++++++++++++++-------
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/sfc/filter.h b/drivers/net/ethernet/sfc/filter.h
index 4d928839d292..be72e71da027 100644
--- a/drivers/net/ethernet/sfc/filter.h
+++ b/drivers/net/ethernet/sfc/filter.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/in6.h>
 #include <asm/byteorder.h>
 
 /**
@@ -223,6 +224,27 @@ efx_filter_set_ipv4_local(struct efx_filter_spec *spec, u8 proto,
 	return 0;
 }
 
+/**
+ * efx_filter_set_ipv6_local - specify IPv6 host, transport protocol and port
+ * @spec: Specification to initialise
+ * @proto: Transport layer protocol number
+ * @host: Local host address (network byte order)
+ * @port: Local port (network byte order)
+ */
+static inline int
+efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
+			  const struct in6_addr *host, __be16 port)
+{
+	spec->match_flags |=
+		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
+		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT;
+	spec->ether_type = htons(ETH_P_IPV6);
+	spec->ip_proto = proto;
+	memcpy(spec->loc_host, host, sizeof(spec->loc_host));
+	spec->loc_port = port;
+	return 0;
+}
+
 /**
  * efx_filter_set_ipv4_full - specify IPv4 hosts, transport protocol and ports
  * @spec: Specification to initialise
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 719005d79943..9b48a5376bbd 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	2
+#define PTP_RXFILTERS_LEN	4
 
-#define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
 
@@ -1297,29 +1299,49 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 	}
 }
 
-static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+static void efx_ptp_init_filter(struct efx_nic *efx,
+				struct efx_filter_spec *rxfilter)
 {
-	struct efx_ptp_data *ptp = efx->ptp_data;
-	struct efx_filter_spec rxfilter;
-	int rc;
+	struct efx_channel *channel = efx->ptp_data->channel;
+	struct efx_rx_queue *queue = efx_channel_get_rx_queue(channel);
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
+	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
+			   efx_rx_queue_index(queue));
+}
 
-	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
-				  htons(port));
+static int efx_ptp_insert_filter(struct efx_nic *efx,
+				 struct efx_filter_spec *rxfilter)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	int rc = efx_filter_insert_filter(efx, rxfilter, true);
 	if (rc < 0)
 		return rc;
-
 	ptp->rxfilters[ptp->rxfilters_count] = rc;
 	ptp->rxfilters_count++;
-
 	return 0;
 }
 
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+{
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDR_IPV4),
+				  htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
+static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
+{
+	const struct in6_addr addr = {{PTP_ADDR_IPV6}};
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv6_local(&rxfilter, IPPROTO_UDP, &addr, htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1336,6 +1358,19 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 	if (rc < 0)
 		goto fail;
 
+	/* if the NIC supports hw timestamps by the MAC, we can support
+	 * PTP over IPv6
+	 */
+	if (efx_ptp_use_mac_tx_timestamps(efx)) {
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
+		if (rc < 0)
+			goto fail;
+
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
+		if (rc < 0)
+			goto fail;
+	}
+
 	return 0;
 
 fail:
-- 
2.34.1

Re: [PATCH net-next v4 2/3] sfc: support PTP over IPv6/UDP

[Edward Cree]

On 31/08/2022 11:16, Íñigo Huguet wrote:
> commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
> PTP") added support for hardware timestamping on TX for cards of the
> 8000 series and newer, in an effort to provide support for other
> transports other than IPv4/UDP.
> 
> However, timestamping was still not working on RX for these other
> transports. This patch add support for PTP over IPv6/UDP.
> 
> Tested: sync as master and as slave is correct using ptp4l from linuxptp
> package, both with IPv4 and IPv6.
> 
> Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> ---
<snip>> -	rc = efx_filter_insert_filter(efx, &rxfilter, true);
> +	int rc = efx_filter_insert_filter(efx, rxfilter, true);
>  	if (rc < 0)
>  		return rc;
> -
>  	ptp->rxfilters[ptp->rxfilters_count] = rc;
>  	ptp->rxfilters_count++;
> -
>  	return 0;
>  }

These whitespace changes seem like churn given that this code was
 added in patch #1.  If respinning maybe make the two consistent?

-ed

[PATCH net-next v4 3/3] sfc: support PTP over Ethernet

[Íñigo Huguet]

The previous patch add support for PTP over IPv6/UDP (only for 8000
series and newer) and this one add support for PTP over 802.3.

Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
and IPv6 still works fine.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 9b48a5376bbd..33bb2cbafb27 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,13 +118,14 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	4
+#define PTP_RXFILTERS_LEN	5
 
 #define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
 #define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
+#define PTP_ADDR_ETHER		{0x01, 0x1b, 0x19, 0, 0, 0} /* 01-1B-19-00-00-00 */
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -1342,6 +1343,18 @@ static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
 	return efx_ptp_insert_filter(efx, &rxfilter);
 }
 
+static int efx_ptp_insert_eth_filter(struct efx_nic *efx)
+{
+	const u8 addr[ETH_ALEN] = PTP_ADDR_ETHER;
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_eth_local(&rxfilter, EFX_FILTER_VID_UNSPEC, addr);
+	rxfilter.match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+	rxfilter.ether_type = htons(ETH_P_1588);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1359,7 +1372,7 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		goto fail;
 
 	/* if the NIC supports hw timestamps by the MAC, we can support
-	 * PTP over IPv6
+	 * PTP over IPv6 and Ethernet
 	 */
 	if (efx_ptp_use_mac_tx_timestamps(efx)) {
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
@@ -1369,6 +1382,10 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
 		if (rc < 0)
 			goto fail;
+
+		rc = efx_ptp_insert_eth_filter(efx);
+		if (rc < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH net-next v4 0/3] sfc: add support for PTP over IPv6 and 802.3

[Richard Cochran]

On Wed, Aug 31, 2022 at 12:16:28PM +0200, Íñigo Huguet wrote:
> Most recent cards (8000 series and newer) had enough hardware support
> for this, but it was not enabled in the driver. The transmission of PTP
> packets over these protocols was already added in commit bd4a2697e5e2
> ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> them was already unsupported so synchronization didn't happen.
> 
> These patches add support for timestamping received packets over
> IPv6/UPD and IEEE802.3.
> 
> v2: fixed weird indentation in efx_ptp_init_filter
> v3: fixed bug caused by usage of htons in PTP_EVENT_PORT definition.
>     It was used in more places, where htons was used too, so using it
>     2 times leave it again in host order. I didn't detected it in my
>     tests because it only affected if timestamping through the MC, but
>     the model I used do it through the MAC. Detected by kernel test
>     robot <lkp@intel.com>
> v4: removed `inline` specifiers from 2 local functions
> 
> Íñigo Huguet (3):
>   sfc: allow more flexible way of adding filters for PTP
>   sfc: support PTP over IPv6/UDP
>   sfc: support PTP over Ethernet
> 
>  drivers/net/ethernet/sfc/filter.h |  22 +++++
>  drivers/net/ethernet/sfc/ptp.c    | 131 ++++++++++++++++++++----------
>  2 files changed, 111 insertions(+), 42 deletions(-)

For the series:

Acked-by: Richard Cochran <richardcochran@gmail.com>

[PATCH net-next v5 0/3] sfc: add support for PTP over IPv6 and 802.3

[Íñigo Huguet]

Most recent cards (8000 series and newer) had enough hardware support
for this, but it was not enabled in the driver. The transmission of PTP
packets over these protocols was already added in commit bd4a2697e5e2
("sfc: use hardware tx timestamps for more than PTP"), but receiving
them was already unsupported so synchronization didn't happen.

These patches add support for timestamping received packets over
IPv6/UPD and IEEE802.3.

v2: fixed weird indentation in efx_ptp_init_filter
v3: fixed bug caused by usage of htons in PTP_EVENT_PORT definition.
    It was used in more places, where htons was used too, so using it
    2 times leave it again in host order. I didn't detected it in my
    tests because it only affected if timestamping through the MC, but
    the model I used do it through the MAC. Detected by kernel test
    robot <lkp@intel.com>
v4: removed `inline` specifiers from 2 local functions
v5: restored deleted comment with useful explanation about packets
    reordering. Deleted useless whitespaces.

Íñigo Huguet (3):
  sfc: allow more flexible way of adding filters for PTP
  sfc: support PTP over IPv6/UDP
  sfc: support PTP over Ethernet

 drivers/net/ethernet/sfc/filter.h |  22 +++++
 drivers/net/ethernet/sfc/ptp.c    | 128 +++++++++++++++++++++---------
 2 files changed, 111 insertions(+), 39 deletions(-)

-- 
2.34.1

[PATCH net-next v5 1/3] sfc: allow more flexible way of adding filters for PTP

[Íñigo Huguet]

In preparation for the support of PTP over IPv6/UDP and Ethernet in next
patches, allow a more flexible way of adding and removing RX filters for
PTP. Right now, only 2 filters are allowed, which are the ones needed
for PTP over IPv4/UDP.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 68 ++++++++++++++++------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 10ad0b93d283..1189ed4ffec2 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,6 +118,8 @@
 
 #define	PTP_MIN_LENGTH		63
 
+#define PTP_RXFILTERS_LEN	2
+
 #define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
@@ -224,9 +226,8 @@ struct efx_ptp_timeset {
  * @work: Work task
  * @reset_required: A serious error has occurred and the PTP task needs to be
  *                  reset (disable, enable).
- * @rxfilter_event: Receive filter when operating
- * @rxfilter_general: Receive filter when operating
- * @rxfilter_installed: Receive filter installed
+ * @rxfilters: Receive filters when operating
+ * @rxfilters_count: Num of installed rxfilters, should be == PTP_RXFILTERS_LEN
  * @config: Current timestamp configuration
  * @enabled: PTP operation enabled
  * @mode: Mode in which PTP operating (PTP version)
@@ -295,9 +296,8 @@ struct efx_ptp_data {
 	struct workqueue_struct *workwq;
 	struct work_struct work;
 	bool reset_required;
-	u32 rxfilter_event;
-	u32 rxfilter_general;
-	bool rxfilter_installed;
+	u32 rxfilters[PTP_RXFILTERS_LEN];
+	size_t rxfilters_count;
 	struct hwtstamp_config config;
 	bool enabled;
 	unsigned int mode;
@@ -1290,61 +1290,57 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	if (ptp->rxfilter_installed) {
-		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_general);
+	while (ptp->rxfilters_count) {
+		ptp->rxfilters_count--;
 		efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-					  ptp->rxfilter_event);
-		ptp->rxfilter_installed = false;
+					  ptp->rxfilters[ptp->rxfilters_count]);
 	}
 }
 
-static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
 	struct efx_filter_spec rxfilter;
 	int rc;
 
-	if (!ptp->channel || ptp->rxfilter_installed)
-		return 0;
-
-	/* Must filter on both event and general ports to ensure
-	 * that there is no packet re-ordering.
-	 */
 	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
 			   efx_rx_queue_index(
 				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_EVENT_PORT));
-	if (rc != 0)
-		return rc;
+
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
+				  htons(port));
 
 	rc = efx_filter_insert_filter(efx, &rxfilter, true);
 	if (rc < 0)
 		return rc;
-	ptp->rxfilter_event = rc;
+	ptp->rxfilters[ptp->rxfilters_count] = rc;
+	ptp->rxfilters_count++;
+	return 0;
+}
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
-	rc = efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP,
-				       htonl(PTP_ADDRESS),
-				       htons(PTP_GENERAL_PORT));
-	if (rc != 0)
+static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
+	int rc;
+
+	if (!ptp->channel || ptp->rxfilters_count)
+		return 0;
+
+	/* Must filter on both event and general ports to ensure
+	 * that there is no packet re-ordering.
+	 */
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_EVENT_PORT);
+	if (rc < 0)
 		goto fail;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	rc = efx_ptp_insert_ipv4_filter(efx, PTP_GENERAL_PORT);
 	if (rc < 0)
 		goto fail;
-	ptp->rxfilter_general = rc;
 
-	ptp->rxfilter_installed = true;
 	return 0;
 
 fail:
-	efx_filter_remove_id_safe(efx, EFX_FILTER_PRI_REQUIRED,
-				  ptp->rxfilter_event);
+	efx_ptp_remove_multicast_filters(efx);
 	return rc;
 }
 
-- 
2.34.1

[PATCH net-next v5 2/3] sfc: support PTP over IPv6/UDP

[Íñigo Huguet]

commit bd4a2697e5e2 ("sfc: use hardware tx timestamps for more than
PTP") added support for hardware timestamping on TX for cards of the
8000 series and newer, in an effort to provide support for other
transports other than IPv4/UDP.

However, timestamping was still not working on RX for these other
transports. This patch add support for PTP over IPv6/UDP.

Tested: sync as master and as slave is correct using ptp4l from linuxptp
package, both with IPv4 and IPv6.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/filter.h | 22 +++++++++++
 drivers/net/ethernet/sfc/ptp.c    | 61 +++++++++++++++++++++++++------
 2 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/sfc/filter.h b/drivers/net/ethernet/sfc/filter.h
index 4d928839d292..be72e71da027 100644
--- a/drivers/net/ethernet/sfc/filter.h
+++ b/drivers/net/ethernet/sfc/filter.h
@@ -9,6 +9,7 @@
 
 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/in6.h>
 #include <asm/byteorder.h>
 
 /**
@@ -223,6 +224,27 @@ efx_filter_set_ipv4_local(struct efx_filter_spec *spec, u8 proto,
 	return 0;
 }
 
+/**
+ * efx_filter_set_ipv6_local - specify IPv6 host, transport protocol and port
+ * @spec: Specification to initialise
+ * @proto: Transport layer protocol number
+ * @host: Local host address (network byte order)
+ * @port: Local port (network byte order)
+ */
+static inline int
+efx_filter_set_ipv6_local(struct efx_filter_spec *spec, u8 proto,
+			  const struct in6_addr *host, __be16 port)
+{
+	spec->match_flags |=
+		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
+		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT;
+	spec->ether_type = htons(ETH_P_IPV6);
+	spec->ip_proto = proto;
+	memcpy(spec->loc_host, host, sizeof(spec->loc_host));
+	spec->loc_port = port;
+	return 0;
+}
+
 /**
  * efx_filter_set_ipv4_full - specify IPv4 hosts, transport protocol and ports
  * @spec: Specification to initialise
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 1189ed4ffec2..0f27a08bf2f2 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,9 +118,11 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	2
+#define PTP_RXFILTERS_LEN	4
 
-#define PTP_ADDRESS		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
+#define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
 
@@ -1297,20 +1299,22 @@ static void efx_ptp_remove_multicast_filters(struct efx_nic *efx)
 	}
 }
 
-static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+static void efx_ptp_init_filter(struct efx_nic *efx,
+				struct efx_filter_spec *rxfilter)
 {
-	struct efx_ptp_data *ptp = efx->ptp_data;
-	struct efx_filter_spec rxfilter;
-	int rc;
+	struct efx_channel *channel = efx->ptp_data->channel;
+	struct efx_rx_queue *queue = efx_channel_get_rx_queue(channel);
 
-	efx_filter_init_rx(&rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
-			   efx_rx_queue_index(
-				   efx_channel_get_rx_queue(ptp->channel)));
+	efx_filter_init_rx(rxfilter, EFX_FILTER_PRI_REQUIRED, 0,
+			   efx_rx_queue_index(queue));
+}
 
-	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDRESS),
-				  htons(port));
+static int efx_ptp_insert_filter(struct efx_nic *efx,
+				 struct efx_filter_spec *rxfilter)
+{
+	struct efx_ptp_data *ptp = efx->ptp_data;
 
-	rc = efx_filter_insert_filter(efx, &rxfilter, true);
+	int rc = efx_filter_insert_filter(efx, rxfilter, true);
 	if (rc < 0)
 		return rc;
 	ptp->rxfilters[ptp->rxfilters_count] = rc;
@@ -1318,6 +1322,26 @@ static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
 	return 0;
 }
 
+static int efx_ptp_insert_ipv4_filter(struct efx_nic *efx, u16 port)
+{
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv4_local(&rxfilter, IPPROTO_UDP, htonl(PTP_ADDR_IPV4),
+				  htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
+static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
+{
+	const struct in6_addr addr = {{PTP_ADDR_IPV6}};
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_ipv6_local(&rxfilter, IPPROTO_UDP, &addr, htons(port));
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1337,6 +1361,19 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 	if (rc < 0)
 		goto fail;
 
+	/* if the NIC supports hw timestamps by the MAC, we can support
+	 * PTP over IPv6
+	 */
+	if (efx_ptp_use_mac_tx_timestamps(efx)) {
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
+		if (rc < 0)
+			goto fail;
+
+		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
+		if (rc < 0)
+			goto fail;
+	}
+
 	return 0;
 
 fail:
-- 
2.34.1

[PATCH net-next v5 3/3] sfc: support PTP over Ethernet

[Íñigo Huguet]

The previous patch add support for PTP over IPv6/UDP (only for 8000
series and newer) and this one add support for PTP over 802.3.

Tested: sync as master and as slave is correct with ptp4l. PTP over IPv4
and IPv6 still works fine.

Suggested-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ptp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 0f27a08bf2f2..eaef4a15008a 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -118,13 +118,14 @@
 
 #define	PTP_MIN_LENGTH		63
 
-#define PTP_RXFILTERS_LEN	4
+#define PTP_RXFILTERS_LEN	5
 
 #define PTP_ADDR_IPV4		0xe0000181	/* 224.0.1.129 */
 #define PTP_ADDR_IPV6		{0xff, 0x0e, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 				0, 0x01, 0x81}	/* ff0e::181 */
 #define PTP_EVENT_PORT		319
 #define PTP_GENERAL_PORT	320
+#define PTP_ADDR_ETHER		{0x01, 0x1b, 0x19, 0, 0, 0} /* 01-1B-19-00-00-00 */
 
 /* Annoyingly the format of the version numbers are different between
  * versions 1 and 2 so it isn't possible to simply look for 1 or 2.
@@ -1342,6 +1343,18 @@ static int efx_ptp_insert_ipv6_filter(struct efx_nic *efx, u16 port)
 	return efx_ptp_insert_filter(efx, &rxfilter);
 }
 
+static int efx_ptp_insert_eth_filter(struct efx_nic *efx)
+{
+	const u8 addr[ETH_ALEN] = PTP_ADDR_ETHER;
+	struct efx_filter_spec rxfilter;
+
+	efx_ptp_init_filter(efx, &rxfilter);
+	efx_filter_set_eth_local(&rxfilter, EFX_FILTER_VID_UNSPEC, addr);
+	rxfilter.match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+	rxfilter.ether_type = htons(ETH_P_1588);
+	return efx_ptp_insert_filter(efx, &rxfilter);
+}
+
 static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 {
 	struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1362,7 +1375,7 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		goto fail;
 
 	/* if the NIC supports hw timestamps by the MAC, we can support
-	 * PTP over IPv6
+	 * PTP over IPv6 and Ethernet
 	 */
 	if (efx_ptp_use_mac_tx_timestamps(efx)) {
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_EVENT_PORT);
@@ -1372,6 +1385,10 @@ static int efx_ptp_insert_multicast_filters(struct efx_nic *efx)
 		rc = efx_ptp_insert_ipv6_filter(efx, PTP_GENERAL_PORT);
 		if (rc < 0)
 			goto fail;
+
+		rc = efx_ptp_insert_eth_filter(efx);
+		if (rc < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH net-next v5 0/3] sfc: add support for PTP over IPv6 and 802.3

[Edward Cree]

On 05/09/2022 09:23, Íñigo Huguet wrote:
> Most recent cards (8000 series and newer) had enough hardware support
> for this, but it was not enabled in the driver. The transmission of PTP
> packets over these protocols was already added in commit bd4a2697e5e2
> ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> them was already unsupported so synchronization didn't happen.
> 
> These patches add support for timestamping received packets over
> IPv6/UPD and IEEE802.3.
> 
> v2: fixed weird indentation in efx_ptp_init_filter
> v3: fixed bug caused by usage of htons in PTP_EVENT_PORT definition.
>     It was used in more places, where htons was used too, so using it
>     2 times leave it again in host order. I didn't detected it in my
>     tests because it only affected if timestamping through the MC, but
>     the model I used do it through the MAC. Detected by kernel test
>     robot <lkp@intel.com>
> v4: removed `inline` specifiers from 2 local functions
> v5: restored deleted comment with useful explanation about packets
>     reordering. Deleted useless whitespaces.
> 
> Íñigo Huguet (3):
>   sfc: allow more flexible way of adding filters for PTP
>   sfc: support PTP over IPv6/UDP
>   sfc: support PTP over Ethernet
> 
>  drivers/net/ethernet/sfc/filter.h |  22 +++++
>  drivers/net/ethernet/sfc/ptp.c    | 128 +++++++++++++++++++++---------
>  2 files changed, 111 insertions(+), 39 deletions(-)

For the series:
Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

(and thanks for doing this work :)
-ed

Re: [PATCH net-next v5 0/3] sfc: add support for PTP over IPv6 and 802.3

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon,  5 Sep 2022 10:23:20 +0200 you wrote:
> Most recent cards (8000 series and newer) had enough hardware support
> for this, but it was not enabled in the driver. The transmission of PTP
> packets over these protocols was already added in commit bd4a2697e5e2
> ("sfc: use hardware tx timestamps for more than PTP"), but receiving
> them was already unsupported so synchronization didn't happen.
> 
> These patches add support for timestamping received packets over
> IPv6/UPD and IEEE802.3.
> 
> [...]

Here is the summary with links:
  - [net-next,v5,1/3] sfc: allow more flexible way of adding filters for PTP
    https://git.kernel.org/netdev/net-next/c/313aa13a0717
  - [net-next,v5,2/3] sfc: support PTP over IPv6/UDP
    https://git.kernel.org/netdev/net-next/c/621918c45fdc
  - [net-next,v5,3/3] sfc: support PTP over Ethernet
    https://git.kernel.org/netdev/net-next/c/e4616f64726b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html