Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 30 Dec 2016 12:02:16 +0000 (UTC)
From:      Andrew Rybchenko <arybchik@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r310816 - head/sys/dev/sfxge/common
Message-ID:  <201612301202.uBUC2GiB067845@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: arybchik
Date: Fri Dec 30 12:02:16 2016
New Revision: 310816
URL: https://svnweb.freebsd.org/changeset/base/310816

Log:
  sfxge(4): fix efx_filter_supported_filters API
  
  The previous API had various problems, including the length of the
  caller provided buffer not being specified, no means being available
  to discover how big the buffer needs to be, and a lack of clarity of
  what the resulting list contains.
  
  To fix it:
  - add the buffer length as a parameter
  - if the provided buffer is too short, fail with ENOSPC and return the
    required length
  - ensure that the list contents are valid and add comments
    describing it
  
  It is safe to change this API as, unsuprisingly, it has no users.
  
  Submitted by:   Mark Spender <mspender at solarflare.com>
  Reviewed by:    gnn
  Sponsored by:   Solarflare Communications, Inc.
  MFC after:      2 days
  Differential Revision:  https://reviews.freebsd.org/D8971

Modified:
  head/sys/dev/sfxge/common/ef10_filter.c
  head/sys/dev/sfxge/common/ef10_impl.h
  head/sys/dev/sfxge/common/efx.h
  head/sys/dev/sfxge/common/efx_filter.c
  head/sys/dev/sfxge/common/efx_impl.h

Modified: head/sys/dev/sfxge/common/ef10_filter.c
==============================================================================
--- head/sys/dev/sfxge/common/ef10_filter.c	Fri Dec 30 12:00:17 2016	(r310815)
+++ head/sys/dev/sfxge/common/ef10_filter.c	Fri Dec 30 12:02:16 2016	(r310816)
@@ -871,13 +871,16 @@ fail1:
 
 static	__checkReturn	efx_rc_t
 efx_mcdi_get_parser_disp_info(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length)
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp)
 {
 	efx_mcdi_req_t req;
 	uint8_t payload[MAX(MC_CMD_GET_PARSER_DISP_INFO_IN_LEN,
 			    MC_CMD_GET_PARSER_DISP_INFO_OUT_LENMAX)];
+	size_t matches_count;
+	size_t list_size;
 	efx_rc_t rc;
 
 	(void) memset(payload, 0, sizeof (payload));
@@ -897,25 +900,41 @@ efx_mcdi_get_parser_disp_info(
 		goto fail1;
 	}
 
-	*length = MCDI_OUT_DWORD(req,
+	matches_count = MCDI_OUT_DWORD(req,
 	    GET_PARSER_DISP_INFO_OUT_NUM_SUPPORTED_MATCHES);
 
 	if (req.emr_out_length_used <
-	    MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(*length)) {
+	    MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(matches_count)) {
 		rc = EMSGSIZE;
 		goto fail2;
 	}
 
-	memcpy(list,
-	    MCDI_OUT2(req,
-	    uint32_t,
-	    GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES),
-	    (*length) * sizeof (uint32_t));
+	*list_lengthp = matches_count;
+
+	if (buffer_length < matches_count) {
+		rc = ENOSPC;
+		goto fail3;
+	}
+
+	/*
+	 * Check that the elements in the list in the MCDI response are the size
+	 * we expect, so we can just copy them directly. Any conversion of the
+	 * flags is handled by the caller.
+	 */
 	EFX_STATIC_ASSERT(sizeof (uint32_t) ==
 	    MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_LEN);
 
+	list_size = matches_count *
+		MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_LEN;
+	memcpy(buffer,
+	    MCDI_OUT2(req, uint32_t,
+		    GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES),
+	    list_size);
+
 	return (0);
 
+fail3:
+	EFSYS_PROBE(fail3);
 fail2:
 	EFSYS_PROBE(fail2);
 fail1:
@@ -926,14 +945,55 @@ fail1:
 
 	__checkReturn	efx_rc_t
 ef10_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length)
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp)
 {
-	efx_rc_t rc;
 
-	if ((rc = efx_mcdi_get_parser_disp_info(enp, list, length)) != 0)
+	size_t mcdi_list_length;
+	size_t list_length;
+	uint32_t i;
+	efx_rc_t rc;
+	uint32_t all_filter_flags =
+	    (EFX_FILTER_MATCH_REM_HOST | EFX_FILTER_MATCH_LOC_HOST |
+	    EFX_FILTER_MATCH_REM_MAC | EFX_FILTER_MATCH_REM_PORT |
+	    EFX_FILTER_MATCH_LOC_MAC | EFX_FILTER_MATCH_LOC_PORT |
+	    EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_INNER_VID |
+	    EFX_FILTER_MATCH_OUTER_VID | EFX_FILTER_MATCH_IP_PROTO |
+	    EFX_FILTER_MATCH_UNKNOWN_MCAST_DST |
+	    EFX_FILTER_MATCH_UNKNOWN_UCAST_DST);
+
+	rc = efx_mcdi_get_parser_disp_info(enp, buffer, buffer_length,
+					    &mcdi_list_length);
+	if (rc != 0) {
+		if (rc == ENOSPC) {
+			/* Pass through mcdi_list_length for the list length */
+			*list_lengthp = mcdi_list_length;
+		}
 		goto fail1;
+	}
+
+	/*
+	 * The static assertions in ef10_filter_init() ensure that the values of
+	 * the EFX_FILTER_MATCH flags match those used by MCDI, so they don't
+	 * need to be converted.
+	 *
+	 * In case support is added to MCDI for additional flags, remove any
+	 * matches from the list which include flags we don't support. The order
+	 * of the matches is preserved as they are ordered from highest to
+	 * lowest priority.
+	 */
+	EFSYS_ASSERT(mcdi_list_length <= buffer_length);
+	list_length = 0;
+	for (i = 0; i < mcdi_list_length; i++) {
+		if ((buffer[i] & ~all_filter_flags) == 0) {
+			buffer[list_length] = buffer[i];
+			list_length++;
+		}
+	}
+
+	*list_lengthp = list_length;
 
 	return (0);
 

Modified: head/sys/dev/sfxge/common/ef10_impl.h
==============================================================================
--- head/sys/dev/sfxge/common/ef10_impl.h	Fri Dec 30 12:00:17 2016	(r310815)
+++ head/sys/dev/sfxge/common/ef10_impl.h	Fri Dec 30 12:02:16 2016	(r310816)
@@ -1028,9 +1028,10 @@ ef10_filter_delete(
 
 extern	__checkReturn	efx_rc_t
 ef10_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length);
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp);
 
 extern	__checkReturn	efx_rc_t
 ef10_filter_reconfigure(

Modified: head/sys/dev/sfxge/common/efx.h
==============================================================================
--- head/sys/dev/sfxge/common/efx.h	Fri Dec 30 12:00:17 2016	(r310815)
+++ head/sys/dev/sfxge/common/efx.h	Fri Dec 30 12:02:16 2016	(r310816)
@@ -2312,9 +2312,10 @@ efx_filter_restore(
 
 extern	__checkReturn	efx_rc_t
 efx_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length);
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp);
 
 extern			void
 efx_filter_spec_init_rx(

Modified: head/sys/dev/sfxge/common/efx_filter.c
==============================================================================
--- head/sys/dev/sfxge/common/efx_filter.c	Fri Dec 30 12:00:17 2016	(r310815)
+++ head/sys/dev/sfxge/common/efx_filter.c	Fri Dec 30 12:02:16 2016	(r310816)
@@ -64,9 +64,10 @@ siena_filter_delete(
 
 static	__checkReturn	efx_rc_t
 siena_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length);
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp);
 
 #endif /* EFSYS_OPT_SIENA */
 
@@ -212,11 +213,22 @@ efx_filter_fini(
 	enp->en_mod_flags &= ~EFX_MOD_FILTER;
 }
 
+/*
+ * Query the possible combinations of match flags which can be filtered on.
+ * These are returned as a list, of which each 32 bit element is a bitmask
+ * formed of EFX_FILTER_MATCH flags.
+ *
+ * The combinations are ordered in priority from highest to lowest.
+ *
+ * If the provided buffer is too short to hold the list, the call with fail with
+ * ENOSPC and *list_lengthp will be set to the buffer length required.
+ */
 	__checkReturn	efx_rc_t
 efx_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length)
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp)
 {
 	efx_rc_t rc;
 
@@ -225,11 +237,20 @@ efx_filter_supported_filters(
 	EFSYS_ASSERT3U(enp->en_mod_flags, &, EFX_MOD_FILTER);
 	EFSYS_ASSERT(enp->en_efop->efo_supported_filters != NULL);
 
-	if ((rc = enp->en_efop->efo_supported_filters(enp, list, length)) != 0)
+	if (buffer == NULL) {
+		rc = EINVAL;
 		goto fail1;
+	}
+
+	rc = enp->en_efop->efo_supported_filters(enp, buffer, buffer_length,
+						    list_lengthp);
+	if (rc != 0)
+		goto fail2;
 
 	return (0);
 
+fail2:
+	EFSYS_PROBE(fail2);
 fail1:
 	EFSYS_PROBE1(fail1, efx_rc_t, rc);
 
@@ -1349,23 +1370,20 @@ fail1:
 	return (rc);
 }
 
-#define	MAX_SUPPORTED 4
+#define	SIENA_MAX_SUPPORTED_MATCHES 4
 
 static	__checkReturn	efx_rc_t
 siena_filter_supported_filters(
-	__in		efx_nic_t *enp,
-	__out		uint32_t *list,
-	__out		size_t *length)
-{
-	int index = 0;
-	uint32_t rx_matches[MAX_SUPPORTED];
+	__in				efx_nic_t *enp,
+	__out_ecount(buffer_length)	uint32_t *buffer,
+	__in				size_t buffer_length,
+	__out				size_t *list_lengthp)
+{
+	uint32_t index = 0;
+	uint32_t rx_matches[SIENA_MAX_SUPPORTED_MATCHES];
+	size_t list_length;
 	efx_rc_t rc;
 
-	if (list == NULL) {
-		rc = EINVAL;
-		goto fail1;
-	}
-
 	rx_matches[index++] =
 	    EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
 	    EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT |
@@ -1382,14 +1400,22 @@ siena_filter_supported_filters(
 		rx_matches[index++] = EFX_FILTER_MATCH_LOC_MAC;
 	}
 
-	EFSYS_ASSERT3U(index, <=, MAX_SUPPORTED);
+	EFSYS_ASSERT3U(index, <=, SIENA_MAX_SUPPORTED_MATCHES);
+	list_length = index;
+
+	*list_lengthp = list_length;
+
+	if (buffer_length < list_length) {
+		rc = ENOSPC;
+		goto fail1;
+	}
 
-	*length = index;
-	memcpy(list, rx_matches, *length);
+	memcpy(buffer, rx_matches, list_length * sizeof (rx_matches[0]));
 
 	return (0);
 
 fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
 
 	return (rc);
 }

Modified: head/sys/dev/sfxge/common/efx_impl.h
==============================================================================
--- head/sys/dev/sfxge/common/efx_impl.h	Fri Dec 30 12:00:17 2016	(r310815)
+++ head/sys/dev/sfxge/common/efx_impl.h	Fri Dec 30 12:02:16 2016	(r310816)
@@ -231,7 +231,8 @@ typedef struct efx_filter_ops_s {
 	efx_rc_t	(*efo_add)(efx_nic_t *, efx_filter_spec_t *,
 				   boolean_t may_replace);
 	efx_rc_t	(*efo_delete)(efx_nic_t *, efx_filter_spec_t *);
-	efx_rc_t	(*efo_supported_filters)(efx_nic_t *, uint32_t *, size_t *);
+	efx_rc_t	(*efo_supported_filters)(efx_nic_t *, uint32_t *,
+				   size_t, size_t *);
 	efx_rc_t	(*efo_reconfigure)(efx_nic_t *, uint8_t const *, boolean_t,
 				   boolean_t, boolean_t, boolean_t,
 				   uint8_t const *, uint32_t);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201612301202.uBUC2GiB067845>