* Re: [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT
From: Alexei Starovoitov @ 2018-11-09 19:14 UTC (permalink / raw)
To: Song Liu, David Ahern
Cc: Peter Zijlstra, Netdev, lkml, Kernel Team, ast@kernel.org,
daniel@iogearbox.net, acme@kernel.org
In-Reply-To: <74246BC0-EA66-407E-93E6-0D6E8645F486@fb.com>
On 11/9/18 10:49 AM, Song Liu wrote:
>
>
>> On Nov 9, 2018, at 9:08 AM, David Ahern <dsahern@gmail.com> wrote:
>>
>> On 11/8/18 11:49 AM, Song Liu wrote:
>>> Could you please point me to more information about the use cases you worry
>>> about? I am more than happy to optimize the logic for those use cases.
>>
>> bpf load and unload as just another tracepoint to throw into a set of
>> events that are monitored. As mentioned before auditing the loads and
>> unloads is one example.
>
> For the tracepoint approach, we need similar synchronous logic in perf to
> process BPF load/unload events. If we agree this is the right direction,
> I will modify the set with tracepoints.
we had tracepoints in bpf core. We removed it.
I don't think we'll be going back.
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.
From: Daniel Borkmann @ 2018-11-09 9:38 UTC (permalink / raw)
To: Nitin Hande
Cc: Alexei Starovoitov, Joe Stringer, Martin Lau,
netdev@vger.kernel.org, Jesper Dangaard Brouer, John Fastabend
In-Reply-To: <20181028210245.6420f6c7@cubn>
On 10/29/2018 05:02 AM, Nitin Hande wrote:
>
> This patch proposes to extend the sk_lookup() BPF API to the
> XDP hookpoint. The sk_lookup() helper supports a lookup
> on incoming packet to find the corresponding socket that will
> receive this packet. Current support for this BPF API is
> at the tc hookpoint. This patch will extend this API at XDP
> hookpoint. A XDP program can map the incoming packet to the
> 5-tuple parameter and invoke the API to find the corresponding
> socket structure.
>
> Signed-off-by: Nitin Hande <Nitin.Hande@gmail.com>
Looks good to me, applied to bpf-next, thanks Nitin!
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-09 9:56 UTC (permalink / raw)
To: Saeed Mahameed, dsahern@gmail.com, brouer@redhat.com
Cc: netdev@vger.kernel.org, yoel@kviknet.dk
In-Reply-To: <13d8e510ec3287ac0680dfaa311b10d79353c5e7.camel@mellanox.com>
W dniu 09.11.2018 o 05:52, Saeed Mahameed pisze:
> On Thu, 2018-11-08 at 17:42 -0700, David Ahern wrote:
>> On 11/8/18 5:40 PM, Paweł Staszewski wrote:
>>> W dniu 08.11.2018 o 17:32, David Ahern pisze:
>>>> On 11/8/18 9:27 AM, Paweł Staszewski wrote:
>>>>>>> What hardware is this?
>>>>>>>
>>>>> mellanox connectx 4
>>>>> ethtool -i enp175s0f0
>>>>> driver: mlx5_core
>>>>> version: 5.0-0
>>>>> firmware-version: 12.21.1000 (SM_2001000001033)
>>>>> expansion-rom-version:
>>>>> bus-info: 0000:af:00.0
>>>>> supports-statistics: yes
>>>>> supports-test: yes
>>>>> supports-eeprom-access: no
>>>>> supports-register-dump: no
>>>>> supports-priv-flags: yes
>>>>>
>>>>> ethtool -i enp175s0f1
>>>>> driver: mlx5_core
>>>>> version: 5.0-0
>>>>> firmware-version: 12.21.1000 (SM_2001000001033)
>>>>> expansion-rom-version:
>>>>> bus-info: 0000:af:00.1
>>>>> supports-statistics: yes
>>>>> supports-test: yes
>>>>> supports-eeprom-access: no
>>>>> supports-register-dump: no
>>>>> supports-priv-flags: yes
>>>>>
>>>>>>> Start with:
>>>>>>>
>>>>>>> echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
>>>>>>> cat /sys/kernel/debug/tracing/trace_pipe
>>>>>> cat /sys/kernel/debug/tracing/trace_pipe
>>>>>> <idle>-0 [045] ..s. 68469.467752:
>>>>>> xdp_devmap_xmit:
>>>>>> ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0
>>>>>> drops=1
>>>>>> from_ifindex=4 to_ifindex=5 err=-6
>>>> FIB lookup is good, the redirect is happening, but the mlx5
>>>> driver does
>>>> not like it.
>>>>
>>>> I think the -6 is coming from the mlx5 driver and the packet is
>>>> getting
>>>> dropped. Perhaps this check in mlx5e_xdp_xmit:
>>>>
>>>> if (unlikely(sq_num >= priv->channels.num))
>>>> return -ENXIO;
>>> I removed that part and recompiled - but after running now xdp_fwd
>>> i
>>> have kernel pamic :)
> hh, no please don't do such thing :)
yes - dirty "try" :)
Code back in place :)
>
> It must be because the tx netdev has less tx queues than the rx netdev.
> or the rx netdev rings are bound to a high cpu indexes.
>
> anyway, best practice is to open #cores RX/TX netdev on both sides
>
> ethtool -L enp175s0f0 combined $(nproc)
> ethtool -L enp175s0f1 combined $(nproc)
Ok now it is working.
Time for some tests :)
Thanks
>> Jesper or one of the Mellanox folks needs to respond about the config
>> needed to run XDP with this NIC. I don't have a 40G or 100G card to
>> play
>> with.
>
^ permalink raw reply
* [PATCH net 1/1] bnx2x: Assign unique DMAE channel number for FW DMAE transactions.
From: Sudarsana Reddy Kalluru @ 2018-11-09 10:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Michal.Kalderon
Driver assigns DMAE channel 0 for FW as part of START_RAMROD command. FW
uses this channel for DMAE operations (e.g., TIME_SYNC implementation).
Driver also uses the same channel 0 for DMAE operations for some of the PFs
(e.g., PF0 on Port0). This could lead to concurrent access to the DMAE
channel by FW and driver which is not legal. Hence need to assign unique
DMAE id for FW.
Currently following DMAE channels are used by the clients,
MFW - OCBB/OCSD functionality uses DMAE channel 14/15
Driver 0-3 and 8-11 (for PF dmae operations)
4 and 12 (for stats requests)
Assigning unique dmae_id '13' to the FW.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 7 +++++++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 3 +++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 1 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h | 3 +++
4 files changed, 14 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index be15061..0de487a 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -2191,6 +2191,13 @@ void bnx2x_igu_clear_sb_gen(struct bnx2x *bp, u8 func, u8 idu_sb_id,
#define PMF_DMAE_C(bp) (BP_PORT(bp) * MAX_DMAE_C_PER_PORT + \
E1HVN_MAX)
+/* Following is the DMAE channel number allocation for the clients.
+ * MFW: OCBB/OCSD implementations use DMAE channels 14/15 respectively.
+ * Driver: 0-3 and 8-11 (for PF dmae operations)
+ * 4 and 12 (for stats requests)
+ */
+#define BNX2X_FW_DMAE_C 13 /* Channel for FW DMAE operations */
+
/* PCIE link and speed */
#define PCICFG_LINK_WIDTH 0x1f00000
#define PCICFG_LINK_WIDTH_SHIFT 20
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index 142bc11..b9059f4 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -965,6 +965,9 @@ static inline int bnx2x_func_start(struct bnx2x *bp)
start_params->network_cos_mode = STATIC_COS;
else /* CHIP_IS_E1X */
start_params->network_cos_mode = FW_WRR;
+
+ start_params->dmae_cmd_id = BNX2X_FW_DMAE_C;
+
if (bp->udp_tunnel_ports[BNX2X_UDP_PORT_VXLAN].count) {
port = bp->udp_tunnel_ports[BNX2X_UDP_PORT_VXLAN].dst_port;
start_params->vxlan_dst_port = port;
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
index 3f4d2c8..1a33017 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
@@ -6149,6 +6149,7 @@ static inline int bnx2x_func_send_start(struct bnx2x *bp,
rdata->sd_vlan_tag = cpu_to_le16(start_params->sd_vlan_tag);
rdata->path_id = BP_PATH(bp);
rdata->network_cos_mode = start_params->network_cos_mode;
+ rdata->dmae_cmd_id = start_params->dmae_cmd_id;
rdata->vxlan_dst_port = cpu_to_le16(start_params->vxlan_dst_port);
rdata->geneve_dst_port = cpu_to_le16(start_params->geneve_dst_port);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h
index 0bf2fd4..6cc3301 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h
@@ -1188,6 +1188,9 @@ struct bnx2x_func_start_params {
/* Function cos mode */
u8 network_cos_mode;
+ /* DMAE command id to be used for FW DMAE transactions */
+ u8 dmae_cmd_id;
+
/* UDP dest port for VXLAN */
u16 vxlan_dst_port;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/8] More accurate PHC<->system clock synchronization
From: Miroslav Lichvar @ 2018-11-09 10:14 UTC (permalink / raw)
To: netdev
Cc: Richard Cochran, Jacob Keller, Miroslav Lichvar, Marcelo Tosatti,
Jeff Kirsher, Michael Chan
RFC->v1:
- added new patches
- separated PHC timestamp from ptp_system_timestamp
- fixed memory leak in PTP_SYS_OFFSET_EXTENDED
- changed PTP_SYS_OFFSET_EXTENDED to work with array of arrays
- fixed PTP_SYS_OFFSET_EXTENDED to break correctly from loop
- fixed timecounter updates in drivers
- split gettimex in igb driver
- fixed ptp_read_* functions to be available without
CONFIG_PTP_1588_CLOCK
This series enables a more accurate synchronization between PTP hardware
clocks and the system clock.
The first two patches are minor cleanup/bug fixes.
The third patch adds an extended version of the PTP_SYS_OFFSET ioctl,
which returns three timestamps for each measurement. The idea is to
shorten the interval between the system timestamps to contain just the
reading of the lowest register of the PHC in order to reduce the error
in the measured offset and get a smaller upper bound on the maximum
error.
The fourth patch deprecates the original gettime function.
The remaining patches update the gettime function in order to support
the new ioctl in the e1000e, igb, ixgbe, and tg3 drivers.
Tests with few different NICs in different machines show that:
- with an I219 (e1000e) the measured delay was reduced from 2500 to 1300
ns and the error in the measured offset, when compared to the cross
timestamping supported by the driver, was reduced by a factor of 5
- with an I210 (igb) the delay was reduced from 5100 to 1700 ns
- with an I350 (igb) the delay was reduced from 2300 to 750 ns
- with an X550 (ixgbe) the delay was reduced from 1950 to 650 ns
- with a BCM5720 (tg3) the delay was reduced from 2400 to 1200 ns
Miroslav Lichvar (8):
ptp: reorder declarations in ptp_ioctl()
ptp: check gettime64 return code in PTP_SYS_OFFSET ioctl
ptp: add PTP_SYS_OFFSET_EXTENDED ioctl
ptp: deprecate gettime64() in favor of gettimex64()
e1000e: extend PTP gettime function to read system clock
igb: extend PTP gettime function to read system clock
ixgbe: extend PTP gettime function to read system clock
tg3: extend PTP gettime function to read system clock
drivers/net/ethernet/broadcom/tg3.c | 19 ++++--
drivers/net/ethernet/intel/e1000e/e1000.h | 3 +
drivers/net/ethernet/intel/e1000e/netdev.c | 42 ++++++++++---
drivers/net/ethernet/intel/e1000e/ptp.c | 16 +++--
drivers/net/ethernet/intel/igb/igb_ptp.c | 65 +++++++++++++++++---
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 54 +++++++++++++---
drivers/ptp/ptp_chardev.c | 55 ++++++++++++++---
drivers/ptp/ptp_clock.c | 5 +-
include/linux/ptp_clock_kernel.h | 33 ++++++++++
include/uapi/linux/ptp_clock.h | 12 ++++
10 files changed, 253 insertions(+), 51 deletions(-)
--
2.17.2
^ permalink raw reply
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox