Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/8] ptp: reorder declarations in ptp_ioctl()
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

Reorder declarations of variables as reversed Christmas tree.

Cc: Richard Cochran <richardcochran@gmail.com>
Suggested-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/ptp/ptp_chardev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 2012551d93e0..b54b8158ff8a 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -121,18 +121,18 @@ int ptp_open(struct posix_clock *pc, fmode_t fmode)
 
 long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 {
-	struct ptp_clock_caps caps;
-	struct ptp_clock_request req;
-	struct ptp_sys_offset *sysoff = NULL;
-	struct ptp_sys_offset_precise precise_offset;
-	struct ptp_pin_desc pd;
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+	struct ptp_sys_offset_precise precise_offset;
+	struct system_device_crosststamp xtstamp;
 	struct ptp_clock_info *ops = ptp->info;
+	struct ptp_sys_offset *sysoff = NULL;
+	struct ptp_clock_request req;
+	struct ptp_clock_caps caps;
 	struct ptp_clock_time *pct;
+	unsigned int i, pin_index;
+	struct ptp_pin_desc pd;
 	struct timespec64 ts;
-	struct system_device_crosststamp xtstamp;
 	int enable, err = 0;
-	unsigned int i, pin_index;
 
 	switch (cmd) {
 
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 2/8] ptp: check gettime64 return code in PTP_SYS_OFFSET ioctl
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

If a gettime64 call fails, return the error and avoid copying data back
to user.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/ptp/ptp_chardev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index b54b8158ff8a..3c681bed5703 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -228,7 +228,9 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			pct->sec = ts.tv_sec;
 			pct->nsec = ts.tv_nsec;
 			pct++;
-			ptp->info->gettime64(ptp->info, &ts);
+			err = ptp->info->gettime64(ptp->info, &ts);
+			if (err)
+				goto out;
 			pct->sec = ts.tv_sec;
 			pct->nsec = ts.tv_nsec;
 			pct++;
@@ -281,6 +283,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 		break;
 	}
 
+out:
 	kfree(sysoff);
 	return err;
 }
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 3/8] ptp: add PTP_SYS_OFFSET_EXTENDED ioctl
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Marcelo Tosatti
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

The PTP_SYS_OFFSET ioctl, which can be used to measure the offset
between a PHC and the system clock, includes the total time that the
driver needs to read the PHC timestamp.

This typically involves reading of multiple PCI registers (sometimes in
multiple iterations) and the register that contains the lowest bits of
the timestamp is not read in the middle between the two readings of the
system clock. This asymmetry causes the measured offset to have a
significant error.

Introduce a new ioctl, driver function, and helper functions, which
allow the reading of the lowest register to be isolated from the other
readings in order to reduce the asymmetry. The ioctl returns three
timestamps for each measurement:
- system time right before reading the lowest bits of the PHC timestamp
- PHC time
- system time immediately after reading the lowest bits of the PHC
  timestamp

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/ptp/ptp_chardev.c        | 33 ++++++++++++++++++++++++++++++++
 include/linux/ptp_clock_kernel.h | 31 ++++++++++++++++++++++++++++++
 include/uapi/linux/ptp_clock.h   | 12 ++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 3c681bed5703..aad0d36cf5c0 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -122,10 +122,12 @@ int ptp_open(struct posix_clock *pc, fmode_t fmode)
 long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 {
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+	struct ptp_sys_offset_extended *extoff = NULL;
 	struct ptp_sys_offset_precise precise_offset;
 	struct system_device_crosststamp xtstamp;
 	struct ptp_clock_info *ops = ptp->info;
 	struct ptp_sys_offset *sysoff = NULL;
+	struct ptp_system_timestamp sts;
 	struct ptp_clock_request req;
 	struct ptp_clock_caps caps;
 	struct ptp_clock_time *pct;
@@ -211,6 +213,36 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			err = -EFAULT;
 		break;
 
+	case PTP_SYS_OFFSET_EXTENDED:
+		if (!ptp->info->gettimex64) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+		extoff = memdup_user((void __user *)arg, sizeof(*extoff));
+		if (IS_ERR(extoff)) {
+			err = PTR_ERR(extoff);
+			extoff = NULL;
+			break;
+		}
+		if (extoff->n_samples > PTP_MAX_SAMPLES) {
+			err = -EINVAL;
+			break;
+		}
+		for (i = 0; i < extoff->n_samples; i++) {
+			err = ptp->info->gettimex64(ptp->info, &ts, &sts);
+			if (err)
+				goto out;
+			extoff->ts[i][0].sec = sts.pre_ts.tv_sec;
+			extoff->ts[i][0].nsec = sts.pre_ts.tv_nsec;
+			extoff->ts[i][1].sec = ts.tv_sec;
+			extoff->ts[i][1].nsec = ts.tv_nsec;
+			extoff->ts[i][2].sec = sts.post_ts.tv_sec;
+			extoff->ts[i][2].nsec = sts.post_ts.tv_nsec;
+		}
+		if (copy_to_user((void __user *)arg, extoff, sizeof(*extoff)))
+			err = -EFAULT;
+		break;
+
 	case PTP_SYS_OFFSET:
 		sysoff = memdup_user((void __user *)arg, sizeof(*sysoff));
 		if (IS_ERR(sysoff)) {
@@ -284,6 +316,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 	}
 
 out:
+	kfree(extoff);
 	kfree(sysoff);
 	return err;
 }
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 51349d124ee5..a1ec0448e341 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -39,6 +39,15 @@ struct ptp_clock_request {
 };
 
 struct system_device_crosststamp;
+
+/**
+ * struct ptp_system_timestamp - system time corresponding to a PHC timestamp
+ */
+struct ptp_system_timestamp {
+	struct timespec64 pre_ts;
+	struct timespec64 post_ts;
+};
+
 /**
  * struct ptp_clock_info - decribes a PTP hardware clock
  *
@@ -75,6 +84,14 @@ struct system_device_crosststamp;
  * @gettime64:  Reads the current time from the hardware clock.
  *              parameter ts: Holds the result.
  *
+ * @gettimex64:  Reads the current time from the hardware clock and optionally
+ *               also the system clock.
+ *               parameter ts: Holds the PHC timestamp.
+ *               parameter sts: If not NULL, it holds a pair of timestamps from
+ *               the system clock. The first reading is made right before
+ *               reading the lowest bits of the PHC timestamp and the second
+ *               reading immediately follows that.
+ *
  * @getcrosststamp:  Reads the current time from the hardware clock and
  *                   system clock simultaneously.
  *                   parameter cts: Contains timestamp (device,system) pair,
@@ -124,6 +141,8 @@ struct ptp_clock_info {
 	int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
 	int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
 	int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
+	int (*gettimex64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
+			  struct ptp_system_timestamp *sts);
 	int (*getcrosststamp)(struct ptp_clock_info *ptp,
 			      struct system_device_crosststamp *cts);
 	int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
@@ -247,4 +266,16 @@ static inline int ptp_schedule_worker(struct ptp_clock *ptp,
 
 #endif
 
+static inline void ptp_read_system_prets(struct ptp_system_timestamp *sts)
+{
+	if (sts)
+		ktime_get_real_ts64(&sts->pre_ts);
+}
+
+static inline void ptp_read_system_postts(struct ptp_system_timestamp *sts)
+{
+	if (sts)
+		ktime_get_real_ts64(&sts->post_ts);
+}
+
 #endif
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 3039bf6a742e..d73d83950265 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -84,6 +84,16 @@ struct ptp_sys_offset {
 	struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
 };
 
+struct ptp_sys_offset_extended {
+	unsigned int n_samples; /* Desired number of measurements. */
+	unsigned int rsv[3];    /* Reserved for future use. */
+	/*
+	 * Array of [system, phc, system] time stamps. The kernel will provide
+	 * 3*n_samples time stamps.
+	 */
+	struct ptp_clock_time ts[PTP_MAX_SAMPLES][3];
+};
+
 struct ptp_sys_offset_precise {
 	struct ptp_clock_time device;
 	struct ptp_clock_time sys_realtime;
@@ -136,6 +146,8 @@ struct ptp_pin_desc {
 #define PTP_PIN_SETFUNC    _IOW(PTP_CLK_MAGIC, 7, struct ptp_pin_desc)
 #define PTP_SYS_OFFSET_PRECISE \
 	_IOWR(PTP_CLK_MAGIC, 8, struct ptp_sys_offset_precise)
+#define PTP_SYS_OFFSET_EXTENDED \
+	_IOW(PTP_CLK_MAGIC, 9, struct ptp_sys_offset_extended)
 
 struct ptp_extts_event {
 	struct ptp_clock_time t; /* Time event occured. */
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 4/8] ptp: deprecate gettime64() in favor of gettimex64()
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

When a driver provides gettimex64(), use it in the PTP_SYS_OFFSET ioctl
and POSIX clock's gettime() instead of gettime64(). Drivers should
provide only one of the functions.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/ptp/ptp_chardev.c        | 5 ++++-
 drivers/ptp/ptp_clock.c          | 5 ++++-
 include/linux/ptp_clock_kernel.h | 2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index aad0d36cf5c0..797fab33bb98 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -260,7 +260,10 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			pct->sec = ts.tv_sec;
 			pct->nsec = ts.tv_nsec;
 			pct++;
-			err = ptp->info->gettime64(ptp->info, &ts);
+			if (ops->gettimex64)
+				err = ops->gettimex64(ops, &ts, NULL);
+			else
+				err = ops->gettime64(ops, &ts);
 			if (err)
 				goto out;
 			pct->sec = ts.tv_sec;
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 5419a89d300e..40fda23e4b05 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -117,7 +117,10 @@ static int ptp_clock_gettime(struct posix_clock *pc, struct timespec64 *tp)
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
 	int err;
 
-	err = ptp->info->gettime64(ptp->info, tp);
+	if (ptp->info->gettimex64)
+		err = ptp->info->gettimex64(ptp->info, tp, NULL);
+	else
+		err = ptp->info->gettime64(ptp->info, tp);
 	return err;
 }
 
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index a1ec0448e341..7121bbe76979 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -82,6 +82,8 @@ struct ptp_system_timestamp {
  *            parameter delta: Desired change in nanoseconds.
  *
  * @gettime64:  Reads the current time from the hardware clock.
+ *              This method is deprecated.  New drivers should implement
+ *              the @gettimex64 method instead.
  *              parameter ts: Holds the result.
  *
  * @gettimex64:  Reads the current time from the hardware clock and optionally
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 5/8] e1000e: extend PTP gettime function to read system clock
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Jeff Kirsher
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

This adds support for the PTP_SYS_OFFSET_EXTENDED ioctl.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h  |  3 ++
 drivers/net/ethernet/intel/e1000e/netdev.c | 42 ++++++++++++++++------
 drivers/net/ethernet/intel/e1000e/ptp.c    | 16 +++++----
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index c760dc72c520..be13227f1697 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -505,6 +505,9 @@ extern const struct e1000_info e1000_es2_info;
 void e1000e_ptp_init(struct e1000_adapter *adapter);
 void e1000e_ptp_remove(struct e1000_adapter *adapter);
 
+u64 e1000e_read_systim(struct e1000_adapter *adapter,
+		       struct ptp_system_timestamp *sts);
+
 static inline s32 e1000_phy_hw_reset(struct e1000_hw *hw)
 {
 	return hw->phy.ops.reset(hw);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 16a73bd9f4cb..59bd587d809d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4319,13 +4319,16 @@ void e1000e_reinit_locked(struct e1000_adapter *adapter)
 /**
  * e1000e_sanitize_systim - sanitize raw cycle counter reads
  * @hw: pointer to the HW structure
- * @systim: time value read, sanitized and returned
+ * @systim: PHC time value read, sanitized and returned
+ * @sts: structure to hold system time before and after reading SYSTIML,
+ * may be NULL
  *
  * Errata for 82574/82583 possible bad bits read from SYSTIMH/L:
  * check to see that the time is incrementing at a reasonable
  * rate and is a multiple of incvalue.
  **/
-static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
+static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim,
+				  struct ptp_system_timestamp *sts)
 {
 	u64 time_delta, rem, temp;
 	u64 systim_next;
@@ -4335,7 +4338,9 @@ static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
 	incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
 	for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
 		/* latch SYSTIMH on read of SYSTIML */
+		ptp_read_system_prets(sts);
 		systim_next = (u64)er32(SYSTIML);
+		ptp_read_system_postts(sts);
 		systim_next |= (u64)er32(SYSTIMH) << 32;
 
 		time_delta = systim_next - systim;
@@ -4353,15 +4358,16 @@ static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
 }
 
 /**
- * e1000e_cyclecounter_read - read raw cycle counter (used by time counter)
- * @cc: cyclecounter structure
+ * e1000e_read_systim - read SYSTIM register
+ * @adapter: board private structure
+ * @sts: structure which will contain system time before and after reading
+ * SYSTIML, may be NULL
  **/
-static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
+u64 e1000e_read_systim(struct e1000_adapter *adapter,
+		       struct ptp_system_timestamp *sts)
 {
-	struct e1000_adapter *adapter = container_of(cc, struct e1000_adapter,
-						     cc);
 	struct e1000_hw *hw = &adapter->hw;
-	u32 systimel, systimeh;
+	u32 systimel, systimel_2, systimeh;
 	u64 systim;
 	/* SYSTIMH latching upon SYSTIML read does not work well.
 	 * This means that if SYSTIML overflows after we read it but before
@@ -4369,11 +4375,15 @@ static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
 	 * will experience a huge non linear increment in the systime value
 	 * to fix that we test for overflow and if true, we re-read systime.
 	 */
+	ptp_read_system_prets(sts);
 	systimel = er32(SYSTIML);
+	ptp_read_system_postts(sts);
 	systimeh = er32(SYSTIMH);
 	/* Is systimel is so large that overflow is possible? */
 	if (systimel >= (u32)0xffffffff - E1000_TIMINCA_INCVALUE_MASK) {
-		u32 systimel_2 = er32(SYSTIML);
+		ptp_read_system_prets(sts);
+		systimel_2 = er32(SYSTIML);
+		ptp_read_system_postts(sts);
 		if (systimel > systimel_2) {
 			/* There was an overflow, read again SYSTIMH, and use
 			 * systimel_2
@@ -4386,11 +4396,23 @@ static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
 	systim |= (u64)systimeh << 32;
 
 	if (adapter->flags2 & FLAG2_CHECK_SYSTIM_OVERFLOW)
-		systim = e1000e_sanitize_systim(hw, systim);
+		systim = e1000e_sanitize_systim(hw, systim, sts);
 
 	return systim;
 }
 
+/**
+ * e1000e_cyclecounter_read - read raw cycle counter (used by time counter)
+ * @cc: cyclecounter structure
+ **/
+static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
+{
+	struct e1000_adapter *adapter = container_of(cc, struct e1000_adapter,
+						     cc);
+
+	return e1000e_read_systim(adapter, NULL);
+}
+
 /**
  * e1000_sw_init - Initialize general software structures (struct e1000_adapter)
  * @adapter: board private structure to initialize
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index e1f821edbc21..1a4c65d9feb4 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -161,14 +161,18 @@ static int e1000e_phc_getcrosststamp(struct ptp_clock_info *ptp,
 #endif/*CONFIG_E1000E_HWTS*/
 
 /**
- * e1000e_phc_gettime - Reads the current time from the hardware clock
+ * e1000e_phc_gettimex - Reads the current time from the hardware clock and
+ *                       system clock
  * @ptp: ptp clock structure
- * @ts: timespec structure to hold the current time value
+ * @ts: timespec structure to hold the current PHC time
+ * @sts: structure to hold the current system time
  *
  * Read the timecounter and return the correct value in ns after converting
  * it into a struct timespec.
  **/
-static int e1000e_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+static int e1000e_phc_gettimex(struct ptp_clock_info *ptp,
+			       struct timespec64 *ts,
+			       struct ptp_system_timestamp *sts)
 {
 	struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
 						     ptp_clock_info);
@@ -177,8 +181,8 @@ static int e1000e_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 
 	spin_lock_irqsave(&adapter->systim_lock, flags);
 
-	/* Use timecounter_cyc2time() to allow non-monotonic SYSTIM readings */
-	cycles = adapter->cc.read(&adapter->cc);
+	/* NOTE: Non-monotonic SYSTIM readings may be returned */
+	cycles = e1000e_read_systim(adapter, sts);
 	ns = timecounter_cyc2time(&adapter->tc, cycles);
 
 	spin_unlock_irqrestore(&adapter->systim_lock, flags);
@@ -258,7 +262,7 @@ static const struct ptp_clock_info e1000e_ptp_clock_info = {
 	.pps		= 0,
 	.adjfreq	= e1000e_phc_adjfreq,
 	.adjtime	= e1000e_phc_adjtime,
-	.gettime64	= e1000e_phc_gettime,
+	.gettimex64	= e1000e_phc_gettimex,
 	.settime64	= e1000e_phc_settime,
 	.enable		= e1000e_phc_enable,
 };
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 6/8] igb: extend PTP gettime function to read system clock
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Jeff Kirsher
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

This adds support for the PTP_SYS_OFFSET_EXTENDED ioctl.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 65 ++++++++++++++++++++----
 1 file changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 29ced6b74d36..8c1833a157d3 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -275,17 +275,53 @@ static int igb_ptp_adjtime_i210(struct ptp_clock_info *ptp, s64 delta)
 	return 0;
 }
 
-static int igb_ptp_gettime_82576(struct ptp_clock_info *ptp,
-				 struct timespec64 *ts)
+static int igb_ptp_gettimex_82576(struct ptp_clock_info *ptp,
+				  struct timespec64 *ts,
+				  struct ptp_system_timestamp *sts)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
 	unsigned long flags;
+	u32 lo, hi;
 	u64 ns;
 
 	spin_lock_irqsave(&igb->tmreg_lock, flags);
 
-	ns = timecounter_read(&igb->tc);
+	ptp_read_system_prets(sts);
+	lo = rd32(E1000_SYSTIML);
+	ptp_read_system_postts(sts);
+	hi = rd32(E1000_SYSTIMH);
+
+	ns = timecounter_cyc2time(&igb->tc, ((u64)hi << 32) | lo);
+
+	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+
+	*ts = ns_to_timespec64(ns);
+
+	return 0;
+}
+
+static int igb_ptp_gettimex_82580(struct ptp_clock_info *ptp,
+				  struct timespec64 *ts,
+				  struct ptp_system_timestamp *sts)
+{
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
+	unsigned long flags;
+	u32 lo, hi;
+	u64 ns;
+
+	spin_lock_irqsave(&igb->tmreg_lock, flags);
+
+	ptp_read_system_prets(sts);
+	rd32(E1000_SYSTIMR);
+	ptp_read_system_postts(sts);
+	lo = rd32(E1000_SYSTIML);
+	hi = rd32(E1000_SYSTIMH);
+
+	ns = timecounter_cyc2time(&igb->tc, ((u64)hi << 32) | lo);
 
 	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
 
@@ -294,16 +330,22 @@ static int igb_ptp_gettime_82576(struct ptp_clock_info *ptp,
 	return 0;
 }
 
-static int igb_ptp_gettime_i210(struct ptp_clock_info *ptp,
-				struct timespec64 *ts)
+static int igb_ptp_gettimex_i210(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts,
+				 struct ptp_system_timestamp *sts)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
 	unsigned long flags;
 
 	spin_lock_irqsave(&igb->tmreg_lock, flags);
 
-	igb_ptp_read_i210(igb, ts);
+	ptp_read_system_prets(sts);
+	rd32(E1000_SYSTIMR);
+	ptp_read_system_postts(sts);
+	ts->tv_nsec = rd32(E1000_SYSTIML);
+	ts->tv_sec = rd32(E1000_SYSTIMH);
 
 	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
 
@@ -656,9 +698,12 @@ static void igb_ptp_overflow_check(struct work_struct *work)
 	struct igb_adapter *igb =
 		container_of(work, struct igb_adapter, ptp_overflow_work.work);
 	struct timespec64 ts;
+	u64 ns;
 
-	igb->ptp_caps.gettime64(&igb->ptp_caps, &ts);
+	/* Update the timecounter */
+	ns = timecounter_read(&igb->tc);
 
+	ts = ns_to_timespec64(ns);
 	pr_debug("igb overflow check at %lld.%09lu\n",
 		 (long long) ts.tv_sec, ts.tv_nsec);
 
@@ -1124,7 +1169,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.pps = 0;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82576;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
-		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
+		adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_82576;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82576;
@@ -1143,7 +1188,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.pps = 0;
 		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
-		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
+		adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_82580;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82580;
@@ -1171,7 +1216,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.pin_config = adapter->sdp_config;
 		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
-		adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
+		adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_i210;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_i210;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
 		adapter->ptp_caps.verify = igb_ptp_verify_pin;
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 7/8] ixgbe: extend PTP gettime function to read system clock
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Jeff Kirsher
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

This adds support for the PTP_SYS_OFFSET_EXTENDED ioctl.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 54 ++++++++++++++++----
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index b3e0d8bb5cbd..d81a50dc9535 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -443,22 +443,52 @@ static int ixgbe_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 }
 
 /**
- * ixgbe_ptp_gettime
+ * ixgbe_ptp_gettimex
  * @ptp: the ptp clock structure
- * @ts: timespec structure to hold the current time value
+ * @ts: timespec to hold the PHC timestamp
+ * @sts: structure to hold the system time before and after reading the PHC
  *
  * read the timecounter and return the correct value on ns,
  * after converting it into a struct timespec.
  */
-static int ixgbe_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+static int ixgbe_ptp_gettimex(struct ptp_clock_info *ptp,
+			      struct timespec64 *ts,
+			      struct ptp_system_timestamp *sts)
 {
 	struct ixgbe_adapter *adapter =
 		container_of(ptp, struct ixgbe_adapter, ptp_caps);
+	struct ixgbe_hw *hw = &adapter->hw;
 	unsigned long flags;
-	u64 ns;
+	u64 ns, stamp;
 
 	spin_lock_irqsave(&adapter->tmreg_lock, flags);
-	ns = timecounter_read(&adapter->hw_tc);
+
+	switch (adapter->hw.mac.type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+	case ixgbe_mac_x550em_a:
+		/* Upper 32 bits represent billions of cycles, lower 32 bits
+		 * represent cycles. However, we use timespec64_to_ns for the
+		 * correct math even though the units haven't been corrected
+		 * yet.
+		 */
+		ptp_read_system_prets(sts);
+		IXGBE_READ_REG(hw, IXGBE_SYSTIMR);
+		ptp_read_system_postts(sts);
+		ts->tv_nsec = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
+		ts->tv_sec = IXGBE_READ_REG(hw, IXGBE_SYSTIMH);
+		stamp = timespec64_to_ns(ts);
+		break;
+	default:
+		ptp_read_system_prets(sts);
+		stamp = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
+		ptp_read_system_postts(sts);
+		stamp |= (u64)IXGBE_READ_REG(hw, IXGBE_SYSTIMH) << 32;
+		break;
+	}
+
+	ns = timecounter_cyc2time(&adapter->hw_tc, stamp);
+
 	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
 
 	*ts = ns_to_timespec64(ns);
@@ -567,10 +597,14 @@ void ixgbe_ptp_overflow_check(struct ixgbe_adapter *adapter)
 {
 	bool timeout = time_is_before_jiffies(adapter->last_overflow_check +
 					     IXGBE_OVERFLOW_PERIOD);
-	struct timespec64 ts;
+	unsigned long flags;
 
 	if (timeout) {
-		ixgbe_ptp_gettime(&adapter->ptp_caps, &ts);
+		/* Update the timecounter */
+		spin_lock_irqsave(&adapter->tmreg_lock, flags);
+		timecounter_read(&adapter->hw_tc);
+		spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
+
 		adapter->last_overflow_check = jiffies;
 	}
 }
@@ -1216,7 +1250,7 @@ static long ixgbe_ptp_create_clock(struct ixgbe_adapter *adapter)
 		adapter->ptp_caps.pps = 1;
 		adapter->ptp_caps.adjfreq = ixgbe_ptp_adjfreq_82599;
 		adapter->ptp_caps.adjtime = ixgbe_ptp_adjtime;
-		adapter->ptp_caps.gettime64 = ixgbe_ptp_gettime;
+		adapter->ptp_caps.gettimex64 = ixgbe_ptp_gettimex;
 		adapter->ptp_caps.settime64 = ixgbe_ptp_settime;
 		adapter->ptp_caps.enable = ixgbe_ptp_feature_enable;
 		adapter->ptp_setup_sdp = ixgbe_ptp_setup_sdp_x540;
@@ -1233,7 +1267,7 @@ static long ixgbe_ptp_create_clock(struct ixgbe_adapter *adapter)
 		adapter->ptp_caps.pps = 0;
 		adapter->ptp_caps.adjfreq = ixgbe_ptp_adjfreq_82599;
 		adapter->ptp_caps.adjtime = ixgbe_ptp_adjtime;
-		adapter->ptp_caps.gettime64 = ixgbe_ptp_gettime;
+		adapter->ptp_caps.gettimex64 = ixgbe_ptp_gettimex;
 		adapter->ptp_caps.settime64 = ixgbe_ptp_settime;
 		adapter->ptp_caps.enable = ixgbe_ptp_feature_enable;
 		break;
@@ -1249,7 +1283,7 @@ static long ixgbe_ptp_create_clock(struct ixgbe_adapter *adapter)
 		adapter->ptp_caps.pps = 0;
 		adapter->ptp_caps.adjfreq = ixgbe_ptp_adjfreq_X550;
 		adapter->ptp_caps.adjtime = ixgbe_ptp_adjtime;
-		adapter->ptp_caps.gettime64 = ixgbe_ptp_gettime;
+		adapter->ptp_caps.gettimex64 = ixgbe_ptp_gettimex;
 		adapter->ptp_caps.settime64 = ixgbe_ptp_settime;
 		adapter->ptp_caps.enable = ixgbe_ptp_feature_enable;
 		adapter->ptp_setup_sdp = NULL;
-- 
2.17.2

^ permalink raw reply related

* [PATCH net-next 8/8] tg3: extend PTP gettime function to read system clock
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
  To: netdev; +Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Michael Chan
In-Reply-To: <20181109101449.15398-1-mlichvar@redhat.com>

This adds support for the PTP_SYS_OFFSET_EXTENDED ioctl.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 89295306f161..ce44d208e137 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6135,10 +6135,16 @@ static int tg3_setup_phy(struct tg3 *tp, bool force_reset)
 }
 
 /* tp->lock must be held */
-static u64 tg3_refclk_read(struct tg3 *tp)
+static u64 tg3_refclk_read(struct tg3 *tp, struct ptp_system_timestamp *sts)
 {
-	u64 stamp = tr32(TG3_EAV_REF_CLCK_LSB);
-	return stamp | (u64)tr32(TG3_EAV_REF_CLCK_MSB) << 32;
+	u64 stamp;
+
+	ptp_read_system_prets(sts);
+	stamp = tr32(TG3_EAV_REF_CLCK_LSB);
+	ptp_read_system_postts(sts);
+	stamp |= (u64)tr32(TG3_EAV_REF_CLCK_MSB) << 32;
+
+	return stamp;
 }
 
 /* tp->lock must be held */
@@ -6229,13 +6235,14 @@ static int tg3_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return 0;
 }
 
-static int tg3_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+static int tg3_ptp_gettimex(struct ptp_clock_info *ptp, struct timespec64 *ts,
+			    struct ptp_system_timestamp *sts)
 {
 	u64 ns;
 	struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
 
 	tg3_full_lock(tp, 0);
-	ns = tg3_refclk_read(tp);
+	ns = tg3_refclk_read(tp, sts);
 	ns += tp->ptp_adjust;
 	tg3_full_unlock(tp);
 
@@ -6330,7 +6337,7 @@ static const struct ptp_clock_info tg3_ptp_caps = {
 	.pps		= 0,
 	.adjfreq	= tg3_ptp_adjfreq,
 	.adjtime	= tg3_ptp_adjtime,
-	.gettime64	= tg3_ptp_gettime,
+	.gettimex64	= tg3_ptp_gettimex,
 	.settime64	= tg3_ptp_settime,
 	.enable		= tg3_ptp_enable,
 };
-- 
2.17.2

^ permalink raw reply related

* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-09 10:20 UTC (permalink / raw)
  To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <6165513d-1e27-31dc-8f94-9de029a73f93@gmail.com>



W dniu 08.11.2018 o 17:06, David Ahern pisze:
> On 11/8/18 6:33 AM, Paweł Staszewski wrote:
>>
>> W dniu 07.11.2018 o 22:06, David Ahern pisze:
>>> On 11/3/18 6:24 PM, Paweł Staszewski wrote:
>>>>> Does your setup have any other device types besides physical ports with
>>>>> VLANs (e.g., any macvlans or bonds)?
>>>>>
>>>>>
>>>> no.
>>>> just
>>>> phy(mlnx)->vlans only config
>>> VLAN and non-VLAN (and a mix) seem to work ok. Patches are here:
>>>      https://github.com/dsahern/linux.git bpf/kernel-tables-wip
>>>
>>> I got lazy with the vlan exports; right now it requires 8021q to be
>>> builtin (CONFIG_VLAN_8021Q=y)
>>>
>>> You can use the xdp_fwd sample:
>>>     make O=kbuild -C samples/bpf -j 8
>>>
>>> Copy samples/bpf/xdp_fwd_kern.o and samples/bpf/xdp_fwd to the server
>>> and run:
>>>      ./xdp_fwd <list of NIC ports>
>>>
>>> e.g., in my testing I run:
>>>      xdp_fwd eth1 eth2 eth3 eth4
>>>
>>> All of the relevant forwarding ports need to be on the same command
>>> line. This version populates a second map to verify the egress port has
>>> XDP enabled.
>> Installed today on some lab server with mellanox connectx4
>>
>> And trying some simple static routing first - but after enabling xdp
>> program - receiver is not receiving frames
>>
>> Route table is simple as possible for tests :)
>>
>> icmp ping test send from 192.168.22.237 to 172.16.0.2 - incomming
>> packets on vlan 4081
>>
>> ip r
>> default via 192.168.22.236 dev vlan4081
>> 172.16.0.0/30 dev vlan1740 proto kernel scope link src 172.16.0.1
>> 192.168.22.0/24 dev vlan4081 proto kernel scope link src 192.168.22.205
>>
>> neigh table:
>> ip neigh ls
>>
>> 192.168.22.237 dev vlan4081 lladdr 00:25:90:fb:a6:8d REACHABLE
>> 172.16.0.2 dev vlan1740 lladdr ac:1f:6b:2c:2e:5a REACHABLE
>>
>> and interfaces:
>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
>> UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
>> UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp/id:5 qdisc
>> mq state UP group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>      inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>>         valid_lft forever preferred_lft forever
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>      inet 192.168.22.205/24 scope global vlan4081
>>         valid_lft forever preferred_lft forever
>>      inet6 fe80::ae1f:6bff:fe07:c890/64 scope link
>>         valid_lft forever preferred_lft forever
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>      inet 172.16.0.1/30 scope global vlan1740
>>         valid_lft forever preferred_lft forever
>>      inet6 fe80::ae1f:6bff:fe07:c891/64 scope link
>>         valid_lft forever preferred_lft forever
>>
>>
>> xdp program detached:
>> Receiving side tcpdump:
>> 14:28:09.141233 IP 192.168.22.237 > 172.16.0.2: ICMP echo request, id
>> 30227, seq 487, length 64
>>
>> I can see icmp requests
>>
>>
>> enabling xdp
>> ./xdp_fwd enp175s0f1 enp175s0f0
>>
>> 4: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>> state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>>      prog/xdp id 5 tag 3c231ff1e5e77f3f
>> 5: enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc mq
>> state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>      prog/xdp id 5 tag 3c231ff1e5e77f3f
>> 6: vlan4081@enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:90 brd ff:ff:ff:ff:ff:ff
>> 7: vlan1740@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc
>> noqueue state UP mode DEFAULT group default qlen 1000
>>      link/ether ac:1f:6b:07:c8:91 brd ff:ff:ff:ff:ff:ff
>>
> What hardware is this?
>
> Start with:
>
> echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
> cat /sys/kernel/debug/tracing/trace_pipe
>
> >From there, you can check the FIB lookups:
> sysctl -w kernel.perf_event_max_stack=16
> perf record -e fib:* -a -g -- sleep 5
> perf script
>

I just catch some weird behavior :)
All was working fine for about 20k packets

Then after xdp start to forward every 10 packets
ping 172.16.0.2 -i 0.1
PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.12 ms
64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.20 ms
64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=4.85 ms
64 bytes from 172.16.0.2: icmp_seq=29 ttl=64 time=4.91 ms
64 bytes from 172.16.0.2: icmp_seq=38 ttl=64 time=4.85 ms
64 bytes from 172.16.0.2: icmp_seq=48 ttl=64 time=5.00 ms
^C
--- 172.16.0.2 ping statistics ---
55 packets transmitted, 6 received, 89% packet loss, time 5655ms
rtt min/avg/max/mdev = 4.850/4.992/5.203/0.145 ms


And again after some time back to normal

  ping 172.16.0.2 -i 0.1
PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.02 ms
64 bytes from 172.16.0.2: icmp_seq=2 ttl=64 time=5.06 ms
64 bytes from 172.16.0.2: icmp_seq=3 ttl=64 time=5.19 ms
64 bytes from 172.16.0.2: icmp_seq=4 ttl=64 time=5.07 ms
64 bytes from 172.16.0.2: icmp_seq=5 ttl=64 time=5.08 ms
64 bytes from 172.16.0.2: icmp_seq=6 ttl=64 time=5.14 ms
64 bytes from 172.16.0.2: icmp_seq=7 ttl=64 time=5.08 ms
64 bytes from 172.16.0.2: icmp_seq=8 ttl=64 time=5.17 ms
64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.04 ms
64 bytes from 172.16.0.2: icmp_seq=10 ttl=64 time=5.10 ms
64 bytes from 172.16.0.2: icmp_seq=11 ttl=64 time=5.11 ms
64 bytes from 172.16.0.2: icmp_seq=12 ttl=64 time=5.13 ms
64 bytes from 172.16.0.2: icmp_seq=13 ttl=64 time=5.12 ms
64 bytes from 172.16.0.2: icmp_seq=14 ttl=64 time=5.15 ms
64 bytes from 172.16.0.2: icmp_seq=15 ttl=64 time=5.13 ms
64 bytes from 172.16.0.2: icmp_seq=16 ttl=64 time=5.04 ms
64 bytes from 172.16.0.2: icmp_seq=17 ttl=64 time=5.12 ms
64 bytes from 172.16.0.2: icmp_seq=18 ttl=64 time=5.07 ms
64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=5.06 ms
64 bytes from 172.16.0.2: icmp_seq=20 ttl=64 time=5.12 ms
64 bytes from 172.16.0.2: icmp_seq=21 ttl=64 time=5.21 ms
64 bytes from 172.16.0.2: icmp_seq=22 ttl=64 time=4.98 ms
^C
--- 172.16.0.2 ping statistics ---
22 packets transmitted, 22 received, 0% packet loss, time 2105ms
rtt min/avg/max/mdev = 4.988/5.104/5.210/0.089 ms


I will try to catch this with debug enabled





Wondering also - cause xdp will bypass now vlan counters and other stuff 
like tcpdump

Is there possible to add only counters from xdp for vlans ?
This will help me in testing.


And also - for non lab scenario there should be possible to sniff 
sometimes on interface :)
Soo wondering if need to attack another xdp program to interface or all 
this can be done by one

I think this is time where i will need to learn more about xdp :)

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Andrew Lunn @ 2018-11-09 20:13 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <fd9674b3-4033-95c6-a93e-10c8b5ba2e6a@gmail.com>

Hi Heiner

> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> +{
> +	return phydrv->config_intr || phydrv->ack_interrupt;
> +}

Should this be && not || ? I thought both needed to be provided for
interrupts to work. 

	   Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: phy: remove flag PHY_HAS_INTERRUPT from driver configs
From: Andrew Lunn @ 2018-11-09 20:13 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <15f15b61-1dc2-e204-0a08-f2d8f73c227d@gmail.com>

On Fri, Nov 09, 2018 at 06:17:22PM +0100, Heiner Kallweit wrote:
> Now that flag PHY_HAS_INTERRUPT has been replaced with a check for
> callbacks config_intr and ack_interrupt, we can remove setting this
> flag from all driver configs.
> Last but not least remove flag PHY_HAS_INTERRUPT completely.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Thanks for removing them all.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Heiner Kallweit @ 2018-11-09 20:22 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <20181109201307.GV5259@lunn.ch>

On 09.11.2018 21:13, Andrew Lunn wrote:
> Hi Heiner
> 
>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
>> +{
>> +	return phydrv->config_intr || phydrv->ack_interrupt;
>> +}
> 
> Should this be && not || ? I thought both needed to be provided for
> interrupts to work. 
> 
> 	   Andrew
> 
I've seen at least one driver which configures interrupts in
config_init and doesn't define a config_intr callback
(ack_interrupt callback is there)
Intention of this check is not to ensure that the driver defines
everything to make interrupts work. All it states:
If at least one of the irq-related callbacks is defined, then
we interpret this as indicator that the PHY supports interrupts.

Heiner

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Florian Fainelli @ 2018-11-09 20:33 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <44b503b8-9f2a-50ac-c4c9-d25258d98ef5@gmail.com>

On 11/9/18 12:22 PM, Heiner Kallweit wrote:
> On 09.11.2018 21:13, Andrew Lunn wrote:
>> Hi Heiner
>>
>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
>>> +{
>>> +	return phydrv->config_intr || phydrv->ack_interrupt;
>>> +}
>>
>> Should this be && not || ? I thought both needed to be provided for
>> interrupts to work. 
>>
>> 	   Andrew
>>
> I've seen at least one driver which configures interrupts in
> config_init and doesn't define a config_intr callback
> (ack_interrupt callback is there)

That driver should probably be fixed, while it most likely does not make
any significant difference during probe/connect, since config_init() and
config_intr() are virtually happening at the same time, this is not
necessarily true when disconnecting from the PHY where we really want
config_intr() to effectively disable the interrupts and not leaving
something enabled that would now become unmaskable, because no more
driver attached.

> Intention of this check is not to ensure that the driver defines
> everything to make interrupts work. All it states:
> If at least one of the irq-related callbacks is defined, then
> we interpret this as indicator that the PHY supports interrupts.

I agree with Andrew here, that this should be an AND here, both
callbacks must be implemented for interrupts to work correctly.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Andrew Lunn @ 2018-11-09 20:38 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <44b503b8-9f2a-50ac-c4c9-d25258d98ef5@gmail.com>

On Fri, Nov 09, 2018 at 09:22:55PM +0100, Heiner Kallweit wrote:
> On 09.11.2018 21:13, Andrew Lunn wrote:
> > Hi Heiner
> > 
> >> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> >> +{
> >> +	return phydrv->config_intr || phydrv->ack_interrupt;
> >> +}
> > 
> > Should this be && not || ? I thought both needed to be provided for
> > interrupts to work. 
> > 
> > 	   Andrew
> > 
> I've seen at least one driver which configures interrupts in
> config_init and doesn't define a config_intr callback
> (ack_interrupt callback is there)

> Intention of this check is not to ensure that the driver defines
> everything to make interrupts work. All it states:
> If at least one of the irq-related callbacks is defined, then
> we interpret this as indicator that the PHY supports interrupts.

I'm just wondering if that driver is broken if it enables interrupts
in config_init()? phylib deliberately enable/disable interrupts. If we
cannot do that, can we get an interrupt when we don't expect it? Can
we miss a state transition which would be reported when interrupts
would be re-enabled immediately triggering an interrupt?

Well, the current code does not seem to care if one is missing. So i
doubt this is making it more broken.

So,

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time
From: Arnd Bergmann @ 2018-11-09 11:02 UTC (permalink / raw)
  To: Sunil Kovvuri; +Cc: Networking, David Miller, linux-soc, Sunil Goutham
In-Reply-To: <CA+sq2CeORXF8kL4gQx9JgfEy-EYMrAr6i5rsUdO0Znw-X5H-fg@mail.gmail.com>

On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> On Fri, Nov 9, 2018 at 2:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:
> > > @@ -666,4 +668,20 @@ struct npc_mcam_unmap_counter_req {
> > >         u8  all;   /* Unmap all entries using this counter ? */
> > >  };
> > >
> > > +struct npc_mcam_alloc_and_write_entry_req {
> > > +       struct mbox_msghdr hdr;
> > > +       struct mcam_entry entry_data;
> > > +       u16 ref_entry;
> > > +       u8  priority;    /* Lower or higher w.r.t ref_entry */
> > > +       u8  intf;        /* Rx or Tx interface */
> > > +       u8  enable_entry;/* Enable this MCAM entry ? */
> > > +       u8  alloc_cntr;  /* Allocate counter and map ? */
> > > +};
> >
> > I noticed that this structure requires padding at the end because
> > struct mbox_msghdr has a 32-bit alignment requirement. For
> > data structures in an interface, I'd recommend avoiding that kind
> > of padding and adding reserved fields or widening the types
> > accordingly.
> >
>
> When there are multiple messages in the mailbox, each message starts
> at a 16byte aligned offset. So struct mbox_msghdr is always aligned.
> I think adding reserved fields is not needed here.
>
> ===
> struct mbox_msghdr *otx2_mbox_alloc_msg_rsp(struct otx2_mbox *mbox, int devid,
>                                             int size, int size_rsp)
> {
>         size = ALIGN(size, MBOX_MSG_ALIGN);
> ===
>
> Is this what you were referring to ?
>

No, I mean the padding at the end of the structure. An example
would be a structure like

struct s {
    u16 a;
    u32 b;
    u16 c;
};

Since b is aligned to four bytes, you get padding between a and b.
On top of that, you also get padding after c to make the size of
structure itself be a multiple of its alignment. For interfaces, we
should avoid both kinds of padding. This can be done by marking
members as __packed (usually I don't recommend that), by
changing the size of members, or by adding explicit 'reserved'
fields in place of the padding.

> > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > use the 'pahole' tool to check for this kind of padding in the
> > API structures.

     Arnd

^ permalink raw reply

* Re: [PATCH 0/4] Add SOCFPGA System Manager
From: Thor Thayer @ 2018-11-09 20:43 UTC (permalink / raw)
  To: Lee Jones
  Cc: peppe.cavallaro, dinguyen, linux, alexandre.torgue, joabreu,
	davem, mchehab+samsung, catalin.marinas, akpm, arnd, aisheng.dong,
	linux-kernel, netdev, linux-arm-kernel
In-Reply-To: <20181019061635.GH4939@dell>

Hi

On 10/19/18 1:16 AM, Lee Jones wrote:
> On Wed, 17 Oct 2018, Thor Thayer wrote:
> 
> 
>> On 10/10/2018 09:42 AM, Thor Thayer wrote:
>>> Hi
>>> On 09/24/2018 05:09 PM, thor.thayer@linux.intel.com wrote:
>>>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>>>
>>>> Add MFD driver for ARM64 SOCFPGA System Manager to steer
>>>> System Manager calls appropriately.
>>>> The SOCFPGA System Manager includes registers from several
>>>> SOC peripherals.
>>>>
>>>> On ARM32, syscon handles this aggregated register grouping.
>>>> Redirect System Manager calls to syscon for ARM32 SOCFPGA
>>>> systems.
>>>>
>>>> The ARM64 System Manager can only be accessed from priority
>>>> level EL3 so this new MFD driver handles the calls to EL3.
>>>>
>>>> Thor Thayer (4):
>>>>     mfd: altera-sysmgr: Add SOCFPGA System Manager abstraction
>>>>     ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
>>>>     arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
>>>>     net: stmmac: socfpga: Convert to shared System Manager driver
>>>>
>>>>    MAINTAINERS                                        |   6 +
>>>>    arch/arm/configs/socfpga_defconfig                 |   1 +
>>>>    arch/arm64/configs/defconfig                       |   1 +
>>>>    drivers/mfd/Kconfig                                |   9 +
>>>>    drivers/mfd/Makefile                               |   1 +
>>>>    drivers/mfd/altera-sysmgr.c                        | 310
>>>> +++++++++++++++++++++
>>>>    .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |   4 +-
>>>>    include/linux/mfd/altera-sysmgr.h                  | 113 ++++++++
>>>>    8 files changed, 444 insertions(+), 1 deletion(-)
>>>>    create mode 100644 drivers/mfd/altera-sysmgr.c
>>>>    create mode 100644 include/linux/mfd/altera-sysmgr.h
>>>>
>>> Gentle ping.
>>
>> Gentle ping again...
>>
>> Any comments on this patch series?
> 
> "Please don't send content free pings and please allow a reasonable time
> for review.  People get busy, go on holiday, attend conferences and so
> on so unless there is some reason for urgency (like critical bug fixes)
> please allow at least a couple of weeks for review.  If there have been
> review comments then people may be waiting for those to be addressed.
> Sending content free pings just adds to the mail volume (if they are
> seen at all) and if something has gone wrong you'll have to resend the
> patches anyway so resending with any comments addressed is generally a
> much better approach."
> 
> In this case, the we are too late in the series to have these
> applied.  Maintainers are generally preparing their submissions for
> the merge-window.  The MFD component of this set is marked as "To
> Review" and I will get around to it when time is more abundant.
> 

Gentle ping now that the merge window is over.

I'm not clear on the scolding for content free pings.

Other pings I've seen just reference the patch series summary instead of 
each patch individually. It seems like pings for each patch would add 
more to the mail volume than the summary but perhaps I'm 
misunderstanding something.  I'm happy to ping each patch separately if 
that is preferable.

Series can be found here:
https://patchwork.kernel.org/cover/10612891/

[PATCH 1/4] mfd: altera-sysmgr: Add SOCFPGA System Manager
[PATCH 2/4] ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
[PATCH 3/4] arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
[PATCH 4/4] net: stmmac: socfpga: Use shared System Manager driver

Thanks,

Thor

^ permalink raw reply

* Re: [PATCH 00/20] octeontx2-af: NPC MCAM support and FLR handling
From: Arnd Bergmann @ 2018-11-09 11:08 UTC (permalink / raw)
  To: Sunil Kovvuri; +Cc: Networking, David Miller, linux-soc, Sunil Goutham
In-Reply-To: <CA+sq2Cf2w1S8Gg+g0DhPU+TCf74U+MHMMWjZvePvAW9wTCNUag@mail.gmail.com>

On Fri, Nov 9, 2018 at 5:35 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Nov 9, 2018 at 2:32 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Nov 8, 2018 at 7:36 PM <sunil.kovvuri@gmail.com> wrote:
> > > From: Sunil Goutham <sgoutham@marvell.com>
> >
> > Hmm, I noticed that you use a different address as the patch author
> > and the submitter. I'm guessing that "Sunil Goutham" and
> > "Sunil Kovvuri" actually refer to the same person, and you just
> > need to pick which of the two email addresses you want to use
> > for public communication, but that's not obvious here.
> >
> > However, if there are actually two different Sunil's here, then
> > you need to add that second Signed-off-by.
> >
>
> No, it's just me.
> Sometimes code indentation becomes messy and difficult to read, if i use
> corporate mail server to submit patches. So i have been using gmail.

Ok, I see. Ideally you should try to get the company mail server fixed
of course. A possible workaround is to add your marvell address as
an alias in gmail, which allows 'git send email' to send out mails with
the other address as the sender. This may however fail if the marvell
mail server uses SPF, as mail clients might then consider your
mails as forged.

        Arnd

^ permalink raw reply

* Re: [PATCH 11/20] octeontx2-af: Add support for stripping STAG/CTAG
From: Arnd Bergmann @ 2018-11-09 11:12 UTC (permalink / raw)
  To: Sunil Kovvuri
  Cc: Networking, David Miller, linux-soc, tduszynski, Sunil Goutham
In-Reply-To: <CA+sq2Cd480FWab9hJKaYxEK2L_HR9oLmqokbqrJwez0FE63txA@mail.gmail.com>

On Fri, Nov 9, 2018 at 5:29 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Nov 9, 2018 at 2:17 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:

> >
> > Here is another instance of bitfields in an interface structure. As
> > before, please try to avoid doing that and use bit shifts and masks
> > instead.
> >
> >        Arnd
>
> No, this struct is not part of communication interface.
> This is used to fill up a register in a bit more readable fashion
> instead of plain bit shifts.

But this is still an interface, isn't it? Writing to the register
implies that there is some hardware that interprets the
bits, so they have to be in the right place.

> ===
> struct nix_rx_vtag_action vtag_action;
>
>         *(u64 *)&vtag_action = 0;
>         vtag_action.vtag0_valid = 1;
>         /* must match type set in NIX_VTAG_CFG */
>         vtag_action.vtag0_type = 0;
>         vtag_action.vtag0_lid = NPC_LID_LA;
>         vtag_action.vtag0_relptr = 12;
>         entry.vtag_action = *(u64 *)&vtag_action;
>
>         /* Set TAG 'action' */
>         rvu_write64(rvu, blkaddr, NPC_AF_MCAMEX_BANKX_TAG_ACT(index, actbank),
>                     entry->vtag_action);

I assume this rvu_write64() does a cpu_to_le64() swap on big-endian,
so the contents again are in the wrong place. I don't see any non-reserved
fields that span an 8-bit boundary, so you can probably rearrange the bits
to make it work, but generally speaking it's better to not rely on how the
compiler lays out bit fields.

        Arnd

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Heiner Kallweit @ 2018-11-09 20:56 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: David Miller, netdev@vger.kernel.org,
	maintainer:BROADCOM BCM63XX ARM ARCHITECTURE, Richard Cochran,
	Carlo Caione, Kevin Hilman, open list,
	moderated list:BROADCOM BCM63XX ARM ARCHITECTURE,
	open list:ARM/Amlogic Meson SoC support
In-Reply-To: <4330cb5b-b1a5-5004-af51-39199943639d@gmail.com>

On 09.11.2018 21:33, Florian Fainelli wrote:
> On 11/9/18 12:22 PM, Heiner Kallweit wrote:
>> On 09.11.2018 21:13, Andrew Lunn wrote:
>>> Hi Heiner
>>>
>>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
>>>> +{
>>>> +	return phydrv->config_intr || phydrv->ack_interrupt;
>>>> +}
>>>
>>> Should this be && not || ? I thought both needed to be provided for
>>> interrupts to work. 
>>>
>>> 	   Andrew
>>>
>> I've seen at least one driver which configures interrupts in
>> config_init and doesn't define a config_intr callback
>> (ack_interrupt callback is there)
> 
> That driver should probably be fixed, while it most likely does not make
> any significant difference during probe/connect, since config_init() and
> config_intr() are virtually happening at the same time, this is not
> necessarily true when disconnecting from the PHY where we really want
> config_intr() to effectively disable the interrupts and not leaving
> something enabled that would now become unmaskable, because no more
> driver attached.
> 
Found the driver: It's the IP101A/G in icplus.c
It should be easy to fix the behavior and move the interrupt config
to a config_intr callback. But the last real changes to the driver
have been done 6 years ago, so I'm not sure there's anybody out
there who can test.

>> Intention of this check is not to ensure that the driver defines
>> everything to make interrupts work. All it states:
>> If at least one of the irq-related callbacks is defined, then
>> we interpret this as indicator that the PHY supports interrupts.
> 
> I agree with Andrew here, that this should be an AND here, both
> callbacks must be implemented for interrupts to work correctly.
> 

^ permalink raw reply

* [PATCH net 0/5] net: aquantia: 2018-11 bugfixes
From: Igor Russkikh @ 2018-11-09 11:53 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh

The patchset fixes a number of bugs found in various areas after
driver validation.

Dmitry Bogdanov (3):
  net: aquantia: fix potential IOMMU fault after driver unbind
  net: aquantia: invalid checksumm offload implementation
  net: aquantia: allow rx checksum offload configuration

Igor Russkikh (2):
  net: aquantia: synchronized flow control between mac/phy
  net: aquantia: fixed enable unicast on 32 macvlan

 .../net/ethernet/aquantia/atlantic/aq_ethtool.c    |  8 +--
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h     |  6 +++
 drivers/net/ethernet/aquantia/atlantic/aq_main.c   | 10 +++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    | 18 +++++--
 drivers/net/ethernet/aquantia/atlantic/aq_nic.h    |  2 +-
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c   | 35 ++++++++-----
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  | 61 ++++++++++++++--------
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c |  8 +++
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h |  3 ++
 .../aquantia/atlantic/hw_atl/hw_atl_llh_internal.h | 18 +++++++
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 21 ++++++++
 11 files changed, 145 insertions(+), 45 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH net 1/5] net: aquantia: synchronized flow control between mac/phy
From: Igor Russkikh @ 2018-11-09 11:53 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1541751718.git.igor.russkikh@aquantia.com>

Flow control statuses were not synchronized between blocks,
that caused packets/link drop on some corner cases, when
MAC sent PFC although Phy was not expecting these to come.

Driver should readout the negotiated FC from phy and
configure RX block accordigly.

This is done on each link change event with information from FW.

Fixes: 288551de45aa ("net: aquantia: Implement rx/tx flow control ethtools callback")

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c |  8 ++++----
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h      |  3 +++
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c     | 14 +++++++++++++-
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c   | 12 +++++++++---
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c    | 21 +++++++++++++++++++++
 5 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index 6a633c70f603..99ef1daaa4d8 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
@@ -407,13 +407,13 @@ static void aq_ethtool_get_pauseparam(struct net_device *ndev,
 				      struct ethtool_pauseparam *pause)
 {
 	struct aq_nic_s *aq_nic = netdev_priv(ndev);
+	u32 fc = aq_nic->aq_nic_cfg.flow_control;
 
 	pause->autoneg = 0;
 
-	if (aq_nic->aq_hw->aq_nic_cfg->flow_control & AQ_NIC_FC_RX)
-		pause->rx_pause = 1;
-	if (aq_nic->aq_hw->aq_nic_cfg->flow_control & AQ_NIC_FC_TX)
-		pause->tx_pause = 1;
+	pause->rx_pause = !!(fc & AQ_NIC_FC_RX);
+	pause->tx_pause = !!(fc & AQ_NIC_FC_TX);
+
 }
 
 static int aq_ethtool_set_pauseparam(struct net_device *ndev,
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
index e8689241204e..7ec8d24b2b0b 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
@@ -204,6 +204,7 @@ struct aq_hw_ops {
 
 	int (*hw_get_fw_version)(struct aq_hw_s *self, u32 *fw_version);
 
+	int (*hw_set_fc)(struct aq_hw_s *self, u32 fc, u32 tc);
 };
 
 struct aq_fw_ops {
@@ -226,6 +227,8 @@ struct aq_fw_ops {
 
 	int (*update_stats)(struct aq_hw_s *self);
 
+	u32 (*get_flow_control)(struct aq_hw_s *self, u32 *fcmode);
+
 	int (*set_flow_control)(struct aq_hw_s *self);
 
 	int (*set_power)(struct aq_hw_s *self, unsigned int power_state,
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 5fed24446687..0011a3f2f672 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -124,6 +124,7 @@ void aq_nic_cfg_start(struct aq_nic_s *self)
 static int aq_nic_update_link_status(struct aq_nic_s *self)
 {
 	int err = self->aq_fw_ops->update_link_status(self->aq_hw);
+	u32 fc = 0;
 
 	if (err)
 		return err;
@@ -133,6 +134,15 @@ static int aq_nic_update_link_status(struct aq_nic_s *self)
 			AQ_CFG_DRV_NAME, self->link_status.mbps,
 			self->aq_hw->aq_link_status.mbps);
 		aq_nic_update_interrupt_moderation_settings(self);
+
+		/* Driver has to update flow control settings on RX block
+		 * on any link event.
+		 * We should query FW whether it negotiated FC.
+		 */
+		if (self->aq_fw_ops->get_flow_control)
+			self->aq_fw_ops->get_flow_control(self->aq_hw, &fc);
+		if (self->aq_hw_ops->hw_set_fc)
+			self->aq_hw_ops->hw_set_fc(self->aq_hw, fc, 0);
 	}
 
 	self->link_status = self->aq_hw->aq_link_status;
@@ -772,7 +782,9 @@ void aq_nic_get_link_ksettings(struct aq_nic_s *self,
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     Pause);
 
-	if (self->aq_nic_cfg.flow_control & AQ_NIC_FC_TX)
+	/* Asym is when either RX or TX, but not both */
+	if (!!(self->aq_nic_cfg.flow_control & AQ_NIC_FC_TX) ^
+	    !!(self->aq_nic_cfg.flow_control & AQ_NIC_FC_RX))
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     Asym_Pause);
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 76d25d594a0f..119265762b0c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -100,12 +100,17 @@ static int hw_atl_b0_hw_reset(struct aq_hw_s *self)
 	return err;
 }
 
+static int hw_atl_b0_set_fc(struct aq_hw_s *self, u32 fc, u32 tc)
+{
+	hw_atl_rpb_rx_xoff_en_per_tc_set(self, !!(fc & AQ_NIC_FC_RX), tc);
+	return 0;
+}
+
 static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 {
 	u32 tc = 0U;
 	u32 buff_size = 0U;
 	unsigned int i_priority = 0U;
-	bool is_rx_flow_control = false;
 
 	/* TPS Descriptor rate init */
 	hw_atl_tps_tx_pkt_shed_desc_rate_curr_time_res_set(self, 0x0U);
@@ -138,7 +143,6 @@ static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 
 	/* QoS Rx buf size per TC */
 	tc = 0;
-	is_rx_flow_control = (AQ_NIC_FC_RX & self->aq_nic_cfg->flow_control);
 	buff_size = HW_ATL_B0_RXBUF_MAX;
 
 	hw_atl_rpb_rx_pkt_buff_size_per_tc_set(self, buff_size, tc);
@@ -150,7 +154,8 @@ static int hw_atl_b0_hw_qos_set(struct aq_hw_s *self)
 						   (buff_size *
 						   (1024U / 32U) * 50U) /
 						   100U, tc);
-	hw_atl_rpb_rx_xoff_en_per_tc_set(self, is_rx_flow_control ? 1U : 0U, tc);
+
+	hw_atl_b0_set_fc(self, self->aq_nic_cfg->flow_control, tc);
 
 	/* QoS 802.1p priority -> TC mapping */
 	for (i_priority = 8U; i_priority--;)
@@ -963,4 +968,5 @@ const struct aq_hw_ops hw_atl_ops_b0 = {
 	.hw_get_regs                 = hw_atl_utils_hw_get_regs,
 	.hw_get_hw_stats             = hw_atl_utils_get_hw_stats,
 	.hw_get_fw_version           = hw_atl_utils_get_fw_version,
+	.hw_set_fc                   = hw_atl_b0_set_fc,
 };
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 096ca5730887..7de3220d9cab 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -30,6 +30,8 @@
 #define HW_ATL_FW2X_MPI_STATE_ADDR	0x370
 #define HW_ATL_FW2X_MPI_STATE2_ADDR	0x374
 
+#define HW_ATL_FW2X_CAP_PAUSE            BIT(CAPS_HI_PAUSE)
+#define HW_ATL_FW2X_CAP_ASYM_PAUSE       BIT(CAPS_HI_ASYMMETRIC_PAUSE)
 #define HW_ATL_FW2X_CAP_SLEEP_PROXY      BIT(CAPS_HI_SLEEP_PROXY)
 #define HW_ATL_FW2X_CAP_WOL              BIT(CAPS_HI_WOL)
 
@@ -451,6 +453,24 @@ static int aq_fw2x_set_flow_control(struct aq_hw_s *self)
 	return 0;
 }
 
+static u32 aq_fw2x_get_flow_control(struct aq_hw_s *self, u32 *fcmode)
+{
+	u32 mpi_state = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
+
+	if (mpi_state & HW_ATL_FW2X_CAP_PAUSE)
+		if (mpi_state & HW_ATL_FW2X_CAP_ASYM_PAUSE)
+			*fcmode = AQ_NIC_FC_RX;
+		else
+			*fcmode = AQ_NIC_FC_RX | AQ_NIC_FC_TX;
+	else
+		if (mpi_state & HW_ATL_FW2X_CAP_ASYM_PAUSE)
+			*fcmode = AQ_NIC_FC_TX;
+		else
+			*fcmode = 0;
+
+	return 0;
+}
+
 const struct aq_fw_ops aq_fw_2x_ops = {
 	.init = aq_fw2x_init,
 	.deinit = aq_fw2x_deinit,
@@ -465,4 +485,5 @@ const struct aq_fw_ops aq_fw_2x_ops = {
 	.set_eee_rate = aq_fw2x_set_eee_rate,
 	.get_eee_rate = aq_fw2x_get_eee_rate,
 	.set_flow_control = aq_fw2x_set_flow_control,
+	.get_flow_control = aq_fw2x_get_flow_control
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH net 2/5] net: aquantia: fix potential IOMMU fault after driver unbind
From: Igor Russkikh @ 2018-11-09 11:53 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1541751718.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

IOMMU fault may occurr on unbind/bind or if_down/if_up sequence.

Although driver disables the rings on down, this is not enough.
Due to internal HW design, during subsequent initialization
NIC sometimes may reuse RX descriptors cache and write to the
host memory from the descriptor cache.
That's get catched by IOMMU on host.

This patch invalidates the descriptor cache in NIC on interface down
to prevent writing to the cached descriptors and to the memory pointed
in those descriptors.

Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 .../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  |  6 ++++++
 .../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c |  8 ++++++++
 .../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h |  3 +++
 .../aquantia/atlantic/hw_atl/hw_atl_llh_internal.h     | 18 ++++++++++++++++++
 4 files changed, 35 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 119265762b0c..3aec56623bf5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -920,6 +920,12 @@ static int hw_atl_b0_hw_interrupt_moderation_set(struct aq_hw_s *self)
 static int hw_atl_b0_hw_stop(struct aq_hw_s *self)
 {
 	hw_atl_b0_hw_irq_disable(self, HW_ATL_B0_INT_MASK);
+
+	/* Invalidate Descriptor Cache to prevent writing to the cached
+	 * descriptors and to the data pointer of those descriptors
+	 */
+	hw_atl_rdm_rx_dma_desc_cache_init_set(self, 1);
+
 	return aq_hw_err_from_flags(self);
 }
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
index be0a3a90dfad..5502ec5f0f69 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
@@ -619,6 +619,14 @@ void hw_atl_rpb_rx_flow_ctl_mode_set(struct aq_hw_s *aq_hw, u32 rx_flow_ctl_mode
 			    HW_ATL_RPB_RX_FC_MODE_SHIFT, rx_flow_ctl_mode);
 }
 
+void hw_atl_rdm_rx_dma_desc_cache_init_set(struct aq_hw_s *aq_hw, u32 init)
+{
+	aq_hw_write_reg_bit(aq_hw, HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_ADR,
+			    HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_MSK,
+			    HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_SHIFT,
+			    init);
+}
+
 void hw_atl_rpb_rx_pkt_buff_size_per_tc_set(struct aq_hw_s *aq_hw,
 					    u32 rx_pkt_buff_size_per_tc, u32 buffer)
 {
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
index 7056c7342afc..41f239928c15 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
@@ -325,6 +325,9 @@ void hw_atl_rpb_rx_pkt_buff_size_per_tc_set(struct aq_hw_s *aq_hw,
 					    u32 rx_pkt_buff_size_per_tc,
 					    u32 buffer);
 
+/* set rdm rx dma descriptor cache init */
+void hw_atl_rdm_rx_dma_desc_cache_init_set(struct aq_hw_s *aq_hw, u32 init);
+
 /* set rx xoff enable (per tc) */
 void hw_atl_rpb_rx_xoff_en_per_tc_set(struct aq_hw_s *aq_hw, u32 rx_xoff_en_per_tc,
 				      u32 buffer);
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
index 716674a9b729..a715fa317b1c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
@@ -293,6 +293,24 @@
 /* default value of bitfield desc{d}_reset */
 #define HW_ATL_RDM_DESCDRESET_DEFAULT 0x0
 
+/* rdm_desc_init_i bitfield definitions
+ * preprocessor definitions for the bitfield rdm_desc_init_i.
+ * port="pif_rdm_desc_init_i"
+ */
+
+/* register address for bitfield rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_ADR 0x00005a00
+/* bitmask for bitfield rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_MSK 0xffffffff
+/* inverted bitmask for bitfield rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_MSKN 0x00000000
+/* lower bit position of bitfield  rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_SHIFT 0
+/* width of bitfield rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_WIDTH 32
+/* default value of bitfield rdm_desc_init_i */
+#define HW_ATL_RDM_RX_DMA_DESC_CACHE_INIT_DEFAULT 0x0
+
 /* rx int_desc_wrb_en bitfield definitions
  * preprocessor definitions for the bitfield "int_desc_wrb_en".
  * port="pif_rdm_int_desc_wrb_en_i"
-- 
2.7.4

^ permalink raw reply related

* [PATCH net 3/5] net: aquantia: fixed enable unicast on 32 macvlan
From: Igor Russkikh @ 2018-11-09 11:53 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1541751718.git.igor.russkikh@aquantia.com>

Fixed a condition mistake due to which macvlans unicast
item number 32 was not added in the unicast filter.

The consequence is that when exactly 32 macvlans are created
on NIC, the last created macvlan receives no traffic because
its MAC was not registered in HW.

Fixes: 94b3b542303f ("net: aquantia: vlan unicast address list correct handling")

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Tested-by: Nikita Danilov <nikita.danilov@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 0011a3f2f672..b5e7c98f424c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -600,7 +600,7 @@ int aq_nic_set_multicast_list(struct aq_nic_s *self, struct net_device *ndev)
 		}
 	}
 
-	if (i > 0 && i < AQ_HW_MULTICAST_ADDRESS_MAX) {
+	if (i > 0 && i <= AQ_HW_MULTICAST_ADDRESS_MAX) {
 		packet_filter |= IFF_MULTICAST;
 		self->mc_list.count = i;
 		self->aq_hw_ops->hw_multicast_list_set(self->aq_hw,
-- 
2.7.4

^ permalink raw reply related

* [PATCH net 4/5] net: aquantia: invalid checksumm offload implementation
From: Igor Russkikh @ 2018-11-09 11:54 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1541751718.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

Packets with marked invalid IP/UDP/TCP checksums were considered as good
by the driver. The error was in a logic, processing offload bits in
RX descriptor.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c   | 35 +++++++++++++--------
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  | 36 +++++++++++-----------
 2 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 3db91446cc67..74550ccc7a20 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -172,6 +172,27 @@ bool aq_ring_tx_clean(struct aq_ring_s *self)
 	return !!budget;
 }
 
+static void aq_rx_checksum(struct aq_ring_s *self,
+			   struct aq_ring_buff_s *buff,
+			   struct sk_buff *skb)
+{
+	if (!(self->aq_nic->ndev->features & NETIF_F_RXCSUM))
+		return;
+
+	if (unlikely(buff->is_cso_err)) {
+		++self->stats.rx.errors;
+		skb->ip_summed = CHECKSUM_NONE;
+		return;
+	}
+	if (buff->is_ip_cso) {
+		__skb_incr_checksum_unnecessary(skb);
+		if (buff->is_udp_cso || buff->is_tcp_cso)
+			__skb_incr_checksum_unnecessary(skb);
+	} else {
+		skb->ip_summed = CHECKSUM_NONE;
+	}
+}
+
 #define AQ_SKB_ALIGN SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
 int aq_ring_rx_clean(struct aq_ring_s *self,
 		     struct napi_struct *napi,
@@ -267,18 +288,8 @@ int aq_ring_rx_clean(struct aq_ring_s *self,
 		}
 
 		skb->protocol = eth_type_trans(skb, ndev);
-		if (unlikely(buff->is_cso_err)) {
-			++self->stats.rx.errors;
-			skb->ip_summed = CHECKSUM_NONE;
-		} else {
-			if (buff->is_ip_cso) {
-				__skb_incr_checksum_unnecessary(skb);
-				if (buff->is_udp_cso || buff->is_tcp_cso)
-					__skb_incr_checksum_unnecessary(skb);
-			} else {
-				skb->ip_summed = CHECKSUM_NONE;
-			}
-		}
+
+		aq_rx_checksum(self, buff, skb);
 
 		skb_set_hash(skb, buff->rss_hash,
 			     buff->is_hash_l4 ? PKT_HASH_TYPE_L4 :
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 3aec56623bf5..179ce12fe4d8 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -660,9 +660,9 @@ static int hw_atl_b0_hw_ring_rx_receive(struct aq_hw_s *self,
 		struct hw_atl_rxd_wb_s *rxd_wb = (struct hw_atl_rxd_wb_s *)
 			&ring->dx_ring[ring->hw_head * HW_ATL_B0_RXD_SIZE];
 
-		unsigned int is_err = 1U;
 		unsigned int is_rx_check_sum_enabled = 0U;
 		unsigned int pkt_type = 0U;
+		u8 rx_stat = 0U;
 
 		if (!(rxd_wb->status & 0x1U)) { /* RxD is not done */
 			break;
@@ -670,35 +670,35 @@ static int hw_atl_b0_hw_ring_rx_receive(struct aq_hw_s *self,
 
 		buff = &ring->buff_ring[ring->hw_head];
 
-		is_err = (0x0000003CU & rxd_wb->status);
+		rx_stat = (0x0000003CU & rxd_wb->status) >> 2;
 
 		is_rx_check_sum_enabled = (rxd_wb->type) & (0x3U << 19);
-		is_err &= ~0x20U; /* exclude validity bit */
 
 		pkt_type = 0xFFU & (rxd_wb->type >> 4);
 
-		if (is_rx_check_sum_enabled) {
-			if (0x0U == (pkt_type & 0x3U))
-				buff->is_ip_cso = (is_err & 0x08U) ? 0U : 1U;
+		if (is_rx_check_sum_enabled & BIT(0) &&
+		    (0x0U == (pkt_type & 0x3U)))
+			buff->is_ip_cso = (rx_stat & BIT(1)) ? 0U : 1U;
 
+		if (is_rx_check_sum_enabled & BIT(1)) {
 			if (0x4U == (pkt_type & 0x1CU))
-				buff->is_udp_cso = buff->is_cso_err ? 0U : 1U;
+				buff->is_udp_cso = (rx_stat & BIT(2)) ? 0U :
+						   !!(rx_stat & BIT(3));
 			else if (0x0U == (pkt_type & 0x1CU))
-				buff->is_tcp_cso = buff->is_cso_err ? 0U : 1U;
-
-			/* Checksum offload workaround for small packets */
-			if (rxd_wb->pkt_len <= 60) {
-				buff->is_ip_cso = 0U;
-				buff->is_cso_err = 0U;
-			}
+				buff->is_tcp_cso = (rx_stat & BIT(2)) ? 0U :
+						   !!(rx_stat & BIT(3));
+		}
+		buff->is_cso_err = !!(rx_stat & 0x6);
+		/* Checksum offload workaround for small packets */
+		if (unlikely(rxd_wb->pkt_len <= 60)) {
+			buff->is_ip_cso = 0U;
+			buff->is_cso_err = 0U;
 		}
-
-		is_err &= ~0x18U;
 
 		dma_unmap_page(ndev, buff->pa, buff->len, DMA_FROM_DEVICE);
 
-		if (is_err || rxd_wb->type & 0x1000U) {
-			/* status error or DMA error */
+		if ((rx_stat & BIT(0)) || rxd_wb->type & 0x1000U) {
+			/* MAC error or DMA error */
 			buff->is_error = 1U;
 		} else {
 			if (self->aq_nic_cfg->is_rss) {
-- 
2.7.4

^ permalink raw reply related

* [PATCH net 5/5] net: aquantia: allow rx checksum offload configuration
From: Igor Russkikh @ 2018-11-09 11:54 UTC (permalink / raw)
  To: David S . Miller; +Cc: Dmitry Bogdanov, netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1541751718.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

RX Checksum offloads could not be configured and ignored netdev features
flag for checksumming.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h            |  3 +++
 drivers/net/ethernet/aquantia/atlantic/aq_main.c          | 10 ++++++++--
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c           |  2 +-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.h           |  2 +-
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c |  7 +++++--
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
index 7ec8d24b2b0b..a1e70da358ca 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
@@ -204,6 +204,9 @@ struct aq_hw_ops {
 
 	int (*hw_get_fw_version)(struct aq_hw_s *self, u32 *fw_version);
 
+	int (*hw_set_offload)(struct aq_hw_s *self,
+			      struct aq_nic_cfg_s *aq_nic_cfg);
+
 	int (*hw_set_fc)(struct aq_hw_s *self, u32 fc, u32 tc);
 };
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index e3ae29e523f0..7c07eef275eb 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -99,8 +99,11 @@ static int aq_ndev_set_features(struct net_device *ndev,
 	struct aq_nic_s *aq_nic = netdev_priv(ndev);
 	struct aq_nic_cfg_s *aq_cfg = aq_nic_get_cfg(aq_nic);
 	bool is_lro = false;
+	int err = 0;
+
+	aq_cfg->features = features;
 
-	if (aq_cfg->hw_features & NETIF_F_LRO) {
+	if (aq_cfg->aq_hw_caps->hw_features & NETIF_F_LRO) {
 		is_lro = features & NETIF_F_LRO;
 
 		if (aq_cfg->is_lro != is_lro) {
@@ -112,8 +115,11 @@ static int aq_ndev_set_features(struct net_device *ndev,
 			}
 		}
 	}
+	if ((aq_nic->ndev->features ^ features) & NETIF_F_RXCSUM)
+		err = aq_nic->aq_hw_ops->hw_set_offload(aq_nic->aq_hw,
+							aq_cfg);
 
-	return 0;
+	return err;
 }
 
 static int aq_ndev_set_mac_address(struct net_device *ndev, void *addr)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index b5e7c98f424c..7abdc0952425 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -118,7 +118,7 @@ void aq_nic_cfg_start(struct aq_nic_s *self)
 	}
 
 	cfg->link_speed_msk &= cfg->aq_hw_caps->link_speed_msk;
-	cfg->hw_features = cfg->aq_hw_caps->hw_features;
+	cfg->features = cfg->aq_hw_caps->hw_features;
 }
 
 static int aq_nic_update_link_status(struct aq_nic_s *self)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
index c1582f4e8e1b..44ec47a3d60a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
@@ -23,7 +23,7 @@ struct aq_vec_s;
 
 struct aq_nic_cfg_s {
 	const struct aq_hw_caps_s *aq_hw_caps;
-	u64 hw_features;
+	u64 features;
 	u32 rxds;		/* rx ring size, descriptors # */
 	u32 txds;		/* tx ring size, descriptors # */
 	u32 vecs;		/* vecs==allocated irqs */
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 179ce12fe4d8..f02592f43fe3 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -234,8 +234,10 @@ static int hw_atl_b0_hw_offload_set(struct aq_hw_s *self,
 	hw_atl_tpo_tcp_udp_crc_offload_en_set(self, 1);
 
 	/* RX checksums offloads*/
-	hw_atl_rpo_ipv4header_crc_offload_en_set(self, 1);
-	hw_atl_rpo_tcp_udp_crc_offload_en_set(self, 1);
+	hw_atl_rpo_ipv4header_crc_offload_en_set(self, !!(aq_nic_cfg->features &
+						 NETIF_F_RXCSUM));
+	hw_atl_rpo_tcp_udp_crc_offload_en_set(self, !!(aq_nic_cfg->features &
+					      NETIF_F_RXCSUM));
 
 	/* LSO offloads*/
 	hw_atl_tdm_large_send_offload_en_set(self, 0xFFFFFFFFU);
@@ -974,5 +976,6 @@ const struct aq_hw_ops hw_atl_ops_b0 = {
 	.hw_get_regs                 = hw_atl_utils_hw_get_regs,
 	.hw_get_hw_stats             = hw_atl_utils_get_hw_stats,
 	.hw_get_fw_version           = hw_atl_utils_get_fw_version,
+	.hw_set_offload              = hw_atl_b0_hw_offload_set,
 	.hw_set_fc                   = hw_atl_b0_set_fc,
 };
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox