* [PATCH v1 net-next 13/15] net: dsa: sja1105: Make HOSTPRIO a kernel config
From: Vladimir Oltean @ 2019-09-02 16:25 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem, vinicius.gomes,
vedang.patel, richardcochran
Cc: weifeng.voon, jiri, m-karicheri2, Jose.Abreu, ilias.apalodimas,
jhs, xiyou.wangcong, kurt.kanzenbach, netdev, Vladimir Oltean
In-Reply-To: <20190902162544.24613-1-olteanv@gmail.com>
Unfortunately with this hardware, there is no way to transmit in-band
QoS hints with management frames (i.e. VLAN PCP is ignored). The traffic
class for these is fixed in the static config (which in turn requires a
reset to change).
With the new ability to add time gates for individual traffic classes,
there is a real danger that the user might unknowingly turn off the
traffic class for PTP, BPDUs, LLDP etc.
So we need to manage this situation the best we can. There isn't any
knob in Linux for this, and changing it at runtime probably isn't worth
it either. So just make the setting loud enough by promoting it to a
Kconfig, which the user can customize to their particular setup.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes since RFC:
- None.
drivers/net/dsa/sja1105/Kconfig | 9 +++++++++
drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/sja1105/Kconfig b/drivers/net/dsa/sja1105/Kconfig
index 55424f39cb0d..4dc873e985e6 100644
--- a/drivers/net/dsa/sja1105/Kconfig
+++ b/drivers/net/dsa/sja1105/Kconfig
@@ -17,6 +17,15 @@ tristate "NXP SJA1105 Ethernet switch family support"
- SJA1105R (Gen. 2, SGMII, No TT-Ethernet)
- SJA1105S (Gen. 2, SGMII, TT-Ethernet)
+config NET_DSA_SJA1105_HOSTPRIO
+ int "Traffic class for management traffic"
+ range 0 7
+ default 7
+ depends on NET_DSA_SJA1105
+ help
+ Configure the traffic class which will be used for management
+ (link-local) traffic sent and received over switch ports.
+
config NET_DSA_SJA1105_PTP
bool "Support for the PTP clock on the NXP SJA1105 Ethernet switch"
depends on NET_DSA_SJA1105
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 4b393782cc84..0c03347b6429 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -388,7 +388,7 @@ static int sja1105_init_general_params(struct sja1105_private *priv)
/* Priority queue for link-local management frames
* (both ingress to and egress from CPU - PTP, STP etc)
*/
- .hostprio = 7,
+ .hostprio = CONFIG_NET_DSA_SJA1105_HOSTPRIO,
.mac_fltres1 = SJA1105_LINKLOCAL_FILTER_A,
.mac_flt1 = SJA1105_LINKLOCAL_FILTER_A_MASK,
.incl_srcpt1 = false,
--
2.17.1
^ permalink raw reply related
* [PATCH v1 net-next 10/15] net: dsa: Pass ndo_setup_tc slave callback to drivers
From: Vladimir Oltean @ 2019-09-02 16:25 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem, vinicius.gomes,
vedang.patel, richardcochran
Cc: weifeng.voon, jiri, m-karicheri2, Jose.Abreu, ilias.apalodimas,
jhs, xiyou.wangcong, kurt.kanzenbach, netdev, Vladimir Oltean
In-Reply-To: <20190902162544.24613-1-olteanv@gmail.com>
DSA currently handles shared block filters (for the classifier-action
qdisc) in the core due to what I believe are simply pragmatic reasons -
hiding the complexity from drivers and offerring a simple API for port
mirroring.
Extend the dsa_slave_setup_tc function by passing all other qdisc
offloads to the driver layer, where the driver may choose what it
implements and how. DSA is simply a pass-through in this case.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes since RFC:
- Removed the unused declaration of struct tc_taprio_qopt_offload.
include/net/dsa.h | 2 ++
net/dsa/slave.c | 12 ++++++++----
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 96acb14ec1a8..541fb514e31d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -515,6 +515,8 @@ struct dsa_switch_ops {
bool ingress);
void (*port_mirror_del)(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror);
+ int (*port_setup_tc)(struct dsa_switch *ds, int port,
+ enum tc_setup_type type, void *type_data);
/*
* Cross-chip operations
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9a88035517a6..75d58229a4bd 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1035,12 +1035,16 @@ static int dsa_slave_setup_tc_block(struct net_device *dev,
static int dsa_slave_setup_tc(struct net_device *dev, enum tc_setup_type type,
void *type_data)
{
- switch (type) {
- case TC_SETUP_BLOCK:
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
+ if (type == TC_SETUP_BLOCK)
return dsa_slave_setup_tc_block(dev, type_data);
- default:
+
+ if (!ds->ops->port_setup_tc)
return -EOPNOTSUPP;
- }
+
+ return ds->ops->port_setup_tc(ds, dp->index, type, type_data);
}
static void dsa_slave_get_stats64(struct net_device *dev,
--
2.17.1
^ permalink raw reply related
* [PATCH v1 net-next 06/15] net: dsa: sja1105: Disallow management xmit during switch reset
From: Vladimir Oltean @ 2019-09-02 16:25 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem, vinicius.gomes,
vedang.patel, richardcochran
Cc: weifeng.voon, jiri, m-karicheri2, Jose.Abreu, ilias.apalodimas,
jhs, xiyou.wangcong, kurt.kanzenbach, netdev, Vladimir Oltean
In-Reply-To: <20190902162544.24613-1-olteanv@gmail.com>
The purpose here is to avoid ptp4l fail due to this condition:
timed out while polling for tx timestamp
increasing tx_timestamp_timeout may correct this issue, but it is likely caused by a driver bug
port 1: send peer delay request failed
So either reset the switch before the management frame was sent, or
after it was timestamped as well, but not in the middle.
The condition may arise either due to a true timeout (i.e. because
re-uploading the static config takes time), or due to the TX timestamp
actually getting lost due to reset. For the former we can increase
tx_timestamp_timeout in userspace, for the latter we need this patch.
Locking all traffic during switch reset does not make sense at all,
though. Forcing all CPU-originated traffic to potentially block waiting
for a sleepable context to send > 800 bytes over SPI is not a good idea.
Flows that are autonomously forwarded by the switch will get dropped
anyway during switch reset no matter what. So just let all other
CPU-originated traffic be dropped as well.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes since RFC:
- None.
drivers/net/dsa/sja1105/sja1105_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index abb22f0a9884..d92f15b3aea9 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1391,6 +1391,8 @@ static int sja1105_static_config_reload(struct sja1105_private *priv)
s64 t12, t34;
int rc, i;
+ mutex_lock(&priv->mgmt_lock);
+
mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
/* Back up the dynamic link speed changed by sja1105_adjust_port_config
@@ -1447,6 +1449,8 @@ static int sja1105_static_config_reload(struct sja1105_private *priv)
goto out;
}
out:
+ mutex_unlock(&priv->mgmt_lock);
+
return rc;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v1 net-next 15/15] net: dsa: sja1105: Implement state machine for TAS with PTP clock source
From: Vladimir Oltean @ 2019-09-02 16:25 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem, vinicius.gomes,
vedang.patel, richardcochran
Cc: weifeng.voon, jiri, m-karicheri2, Jose.Abreu, ilias.apalodimas,
jhs, xiyou.wangcong, kurt.kanzenbach, netdev, Vladimir Oltean
In-Reply-To: <20190902162544.24613-1-olteanv@gmail.com>
Tested using the following bash script and the tc from iproute2-next:
#!/bin/bash
set -e -u -o pipefail
NSEC_PER_SEC="1000000000"
gatemask() {
local tc_list="$1"
local mask=0
for tc in ${tc_list}; do
mask=$((${mask} | (1 << ${tc})))
done
printf "%02x" ${mask}
}
if ! systemctl is-active --quiet ptp4l; then
echo "Please start the ptp4l service"
exit
fi
now=$(phc_ctl /dev/ptp1 get | gawk '/clock time is/ { print $5; }')
# Phase-align the base time to the start of the next second.
sec=$(echo "${now}" | gawk -F. '{ print $1; }')
base_time="$(((${sec} + 1) * ${NSEC_PER_SEC}))"
echo 'file drivers/net/dsa/sja1105/sja1105_tas.c +plm' | \
sudo tee /sys/kernel/debug/dynamic_debug/control
tc qdisc add dev swp5 parent root handle 100 taprio \
num_tc 8 \
map 0 1 2 3 5 6 7 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \
base-time ${base_time} \
sched-entry S $(gatemask 7) 100000 \
sched-entry S $(gatemask "0 1 2 3 4 5 6") 400000 \
clockid CLOCK_TAI flags 2
The "state machine" is a workqueue invoked after each manipulation
command on the PTP clock (reset, adjust time, set time, adjust
frequency) which checks over the state of the time-aware scheduler.
So it is not monitored periodically, only in reaction to a PTP command
typically triggered from a userspace daemon (linuxptp). Otherwise there
is no reason for things to go wrong.
Now that the timecounter/cyclecounter has been replaced with hardware
operations on the PTP clock, the TAS Kconfig now depends upon PTP and
the standalone clocksource operating mode has been removed.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes since RFC:
- Used the "delta" terminology instead of "TAS cycle" to be more
consistent and avoid confusion with the cyclic schedule (of which the
"delta" is only the most granular unit, there is no other connection).
drivers/net/dsa/sja1105/Kconfig | 2 +-
drivers/net/dsa/sja1105/sja1105.h | 2 +
drivers/net/dsa/sja1105/sja1105_ptp.c | 26 +-
drivers/net/dsa/sja1105/sja1105_ptp.h | 13 +
drivers/net/dsa/sja1105/sja1105_spi.c | 4 +
drivers/net/dsa/sja1105/sja1105_tas.c | 426 +++++++++++++++++++++++++-
drivers/net/dsa/sja1105/sja1105_tas.h | 27 ++
7 files changed, 486 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/sja1105/Kconfig b/drivers/net/dsa/sja1105/Kconfig
index 4dc873e985e6..9316a23b7c30 100644
--- a/drivers/net/dsa/sja1105/Kconfig
+++ b/drivers/net/dsa/sja1105/Kconfig
@@ -35,7 +35,7 @@ config NET_DSA_SJA1105_PTP
config NET_DSA_SJA1105_TAS
bool "Support for the Time-Aware Scheduler on NXP SJA1105"
- depends on NET_DSA_SJA1105
+ depends on NET_DSA_SJA1105_PTP
help
This enables support for the TTEthernet-based egress scheduling
engine in the SJA1105 DSA driver, which is controlled using a
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 44f7385c51b5..e8f95b6fadfa 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -40,6 +40,8 @@ struct sja1105_regs {
u64 ptp_control;
u64 ptpclk;
u64 ptpclkrate;
+ u64 ptpclkcorp;
+ u64 ptpschtm;
u64 ptpegr_ts[SJA1105_NUM_PORTS];
u64 pad_mii_tx[SJA1105_NUM_PORTS];
u64 pad_mii_id[SJA1105_NUM_PORTS];
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index ed80278a3521..b037834ff820 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -67,6 +67,8 @@ void sja1105et_ptp_cmd_packing(u8 *buf, struct sja1105_ptp_cmd *cmd,
u64 valid = 1;
sja1105_packing(buf, &valid, 31, 31, size, op);
+ sja1105_packing(buf, &cmd->ptpstrtsch, 30, 30, size, op);
+ sja1105_packing(buf, &cmd->ptpstopsch, 29, 29, size, op);
sja1105_packing(buf, &cmd->resptp, 2, 2, size, op);
sja1105_packing(buf, &cmd->corrclk4ts, 1, 1, size, op);
sja1105_packing(buf, &cmd->ptpclkadd, 0, 0, size, op);
@@ -80,14 +82,16 @@ void sja1105pqrs_ptp_cmd_packing(u8 *buf, struct sja1105_ptp_cmd *cmd,
u64 valid = 1;
sja1105_packing(buf, &valid, 31, 31, size, op);
+ sja1105_packing(buf, &cmd->ptpstrtsch, 30, 30, size, op);
+ sja1105_packing(buf, &cmd->ptpstopsch, 29, 29, size, op);
sja1105_packing(buf, &cmd->resptp, 3, 3, size, op);
sja1105_packing(buf, &cmd->corrclk4ts, 2, 2, size, op);
sja1105_packing(buf, &cmd->ptpclkadd, 0, 0, size, op);
}
-static int sja1105_ptp_commit(struct sja1105_private *priv,
- struct sja1105_ptp_cmd *cmd,
- sja1105_spi_rw_mode_t rw)
+int sja1105_ptp_commit(struct sja1105_private *priv,
+ struct sja1105_ptp_cmd *cmd,
+ sja1105_spi_rw_mode_t rw)
{
const struct sja1105_regs *regs = priv->info->regs;
u8 buf[SJA1105_SIZE_PTP_CMD] = {0};
@@ -222,6 +226,8 @@ int sja1105_ptp_reset(struct sja1105_private *priv)
dev_dbg(priv->ds->dev, "Resetting PTP clock\n");
rc = sja1105_ptp_commit(priv, &cmd, SPI_WRITE);
+ sja1105_tas_clockstep(priv);
+
mutex_unlock(&ptp_data->lock);
return rc;
@@ -291,7 +297,11 @@ int __sja1105_ptp_settime(struct sja1105_private *priv, u64 ns,
return rc;
}
- return sja1105_ptpclkval_write(priv, ticks, ptp_sts);
+ rc = sja1105_ptpclkval_write(priv, ticks, ptp_sts);
+
+ sja1105_tas_clockstep(priv);
+
+ return rc;
}
static int sja1105_ptp_settime(struct ptp_clock_info *ptp,
@@ -331,6 +341,8 @@ static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
&clkrate, 4, NULL);
+ sja1105_tas_adjfreq(priv);
+
mutex_unlock(&priv->ptp_data.lock);
return rc;
@@ -366,7 +378,11 @@ int __sja1105_ptp_adjtime(struct sja1105_private *priv, s64 delta)
return rc;
}
- return sja1105_ptpclkval_write(priv, ticks, NULL);
+ rc = sja1105_ptpclkval_write(priv, ticks, NULL);
+
+ sja1105_tas_clockstep(priv);
+
+ return rc;
}
static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.h b/drivers/net/dsa/sja1105/sja1105_ptp.h
index c24c40115650..da68e5881e5f 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.h
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.h
@@ -29,6 +29,8 @@ enum sja1105_ptp_clk_mode {
};
struct sja1105_ptp_cmd {
+ u64 ptpstrtsch; /* start schedule */
+ u64 ptpstopsch; /* stop schedule */
u64 resptp; /* reset */
u64 corrclk4ts; /* use the corrected clock for timestamps */
u64 ptpclkadd; /* enum sja1105_ptp_clk_mode */
@@ -73,6 +75,10 @@ int __sja1105_ptp_settime(struct sja1105_private *priv, u64 ns,
int __sja1105_ptp_adjtime(struct sja1105_private *priv, s64 delta);
+int sja1105_ptp_commit(struct sja1105_private *priv,
+ struct sja1105_ptp_cmd *cmd,
+ sja1105_spi_rw_mode_t rw);
+
#else
struct sja1105_ptp_cmd;
@@ -135,6 +141,13 @@ static inline int __sja1105_ptp_adjtime(struct sja1105_private *priv, s64 delta)
return 0;
}
+static inline int sja1105_ptp_commit(struct sja1105_private *priv,
+ struct sja1105_ptp_cmd *cmd,
+ sja1105_spi_rw_mode_t rw)
+{
+ return 0;
+}
+
#define sja1105et_ptp_cmd_packing NULL
#define sja1105pqrs_ptp_cmd_packing NULL
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 794cc5077565..f6df050c15ec 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -526,9 +526,11 @@ static struct sja1105_regs sja1105et_regs = {
.rmii_ref_clk = {0x100015, 0x10001C, 0x100023, 0x10002A, 0x100031},
.rmii_ext_tx_clk = {0x100018, 0x10001F, 0x100026, 0x10002D, 0x100034},
.ptpegr_ts = {0xC0, 0xC2, 0xC4, 0xC6, 0xC8},
+ .ptpschtm = 0x12, /* Spans 0x12 to 0x13 */
.ptp_control = 0x17,
.ptpclk = 0x18, /* Spans 0x18 to 0x19 */
.ptpclkrate = 0x1A,
+ .ptpclkcorp = 0x1D,
};
static struct sja1105_regs sja1105pqrs_regs = {
@@ -556,9 +558,11 @@ static struct sja1105_regs sja1105pqrs_regs = {
.rmii_ext_tx_clk = {0x100017, 0x10001D, 0x100023, 0x100029, 0x10002F},
.qlevel = {0x604, 0x614, 0x624, 0x634, 0x644},
.ptpegr_ts = {0xC0, 0xC4, 0xC8, 0xCC, 0xD0},
+ .ptpschtm = 0x13, /* Spans 0x13 to 0x14 */
.ptp_control = 0x18,
.ptpclk = 0x19,
.ptpclkrate = 0x1B,
+ .ptpclkcorp = 0x1E,
};
struct sja1105_info sja1105e_info = {
diff --git a/drivers/net/dsa/sja1105/sja1105_tas.c b/drivers/net/dsa/sja1105/sja1105_tas.c
index 769e1d8e5e8f..ed0c3f00c09d 100644
--- a/drivers/net/dsa/sja1105/sja1105_tas.c
+++ b/drivers/net/dsa/sja1105/sja1105_tas.c
@@ -10,6 +10,11 @@
#define SJA1105_GATE_MASK GENMASK_ULL(SJA1105_NUM_TC - 1, 0)
#define SJA1105_TAS_MAX_DELTA BIT(19)
+#define work_to_sja1105_tas(d) \
+ container_of((d), struct sja1105_tas_data, tas_work)
+#define tas_to_sja1105(d) \
+ container_of((d), struct sja1105_private, tas_data)
+
/* This is not a preprocessor macro because the "ns" argument may or may not be
* s64 at caller side. This ensures it is properly type-cast before div_s64.
*/
@@ -18,6 +23,102 @@ static s64 ns_to_sja1105_delta(s64 ns)
return div_s64(ns, 200);
}
+static s64 sja1105_delta_to_ns(s64 delta)
+{
+ return delta * 200;
+}
+
+/* Calculate the first base_time in the future that satisfies this
+ * relationship:
+ *
+ * future_base_time = base_time + N x cycle_time >= now, or
+ *
+ * now - base_time
+ * N >= ---------------
+ * cycle_time
+ *
+ * Because N is an integer, the ceiling value of the above "a / b" ratio
+ * is in fact precisely the floor value of "(a + b - 1) / b", which is
+ * easier to calculate only having integer division tools.
+ */
+static s64 future_base_time(s64 base_time, s64 cycle_time, s64 now)
+{
+ s64 a, b, n;
+
+ if (base_time >= now)
+ return base_time;
+
+ a = now - base_time;
+ b = cycle_time;
+ n = div_s64(a + b - 1, b);
+
+ return base_time + n * cycle_time;
+}
+
+static int sja1105_tas_set_runtime_params(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+ s64 earliest_base_time = S64_MAX;
+ s64 latest_base_time = 0;
+ s64 its_cycle_time = 0;
+ s64 max_cycle_time = 0;
+ int port;
+
+ tas_data->enabled = false;
+
+ for (port = 0; port < SJA1105_NUM_PORTS; port++) {
+ const struct tc_taprio_qopt_offload *tas_config;
+
+ tas_config = tas_data->config[port];
+ if (!tas_config)
+ continue;
+
+ tas_data->enabled = true;
+
+ if (max_cycle_time < tas_config->cycle_time)
+ max_cycle_time = tas_config->cycle_time;
+ if (latest_base_time < tas_config->base_time)
+ latest_base_time = tas_config->base_time;
+ if (earliest_base_time > tas_config->base_time) {
+ earliest_base_time = tas_config->base_time;
+ its_cycle_time = tas_config->cycle_time;
+ }
+ }
+
+ if (!tas_data->enabled)
+ return 0;
+
+ /* Roll the earliest base time over until it is in a comparable
+ * time base with the latest, then compare their deltas.
+ * We want to enforce that all ports' base times are within
+ * SJA1105_TAS_MAX_DELTA 200ns cycles of one another.
+ */
+ earliest_base_time = future_base_time(earliest_base_time,
+ its_cycle_time,
+ latest_base_time);
+ while (earliest_base_time > latest_base_time)
+ earliest_base_time -= its_cycle_time;
+ if (latest_base_time - earliest_base_time >
+ sja1105_delta_to_ns(SJA1105_TAS_MAX_DELTA)) {
+ dev_err(priv->ds->dev,
+ "Base times too far apart: min %llu max %llu\n",
+ earliest_base_time, latest_base_time);
+ return -ERANGE;
+ }
+
+ tas_data->earliest_base_time = earliest_base_time;
+ tas_data->max_cycle_time = max_cycle_time;
+
+ dev_dbg(priv->ds->dev, "earliest base time %lld ns\n",
+ tas_data->earliest_base_time);
+ dev_dbg(priv->ds->dev, "latest base time %lld ns\n",
+ tas_data->earliest_base_time);
+ dev_dbg(priv->ds->dev, "longest cycle time %lld ns\n",
+ tas_data->max_cycle_time);
+
+ return 0;
+}
+
/* Lo and behold: the egress scheduler from hell.
*
* At the hardware level, the Time-Aware Shaper holds a global linear arrray of
@@ -100,7 +201,11 @@ static int sja1105_init_scheduling(struct sja1105_private *priv)
int num_cycles = 0;
int cycle = 0;
int i, k = 0;
- int port;
+ int port, rc;
+
+ rc = sja1105_tas_set_runtime_params(priv);
+ if (rc < 0)
+ return rc;
/* Discard previous Schedule Table */
table = &priv->static_config.tables[BLK_IDX_SCHEDULE];
@@ -181,11 +286,13 @@ static int sja1105_init_scheduling(struct sja1105_private *priv)
schedule_entry_points = table->entries;
/* Finally start populating the static config tables */
- schedule_entry_points_params->clksrc = SJA1105_TAS_CLKSRC_STANDALONE;
+ schedule_entry_points_params->clksrc = SJA1105_TAS_CLKSRC_PTP;
schedule_entry_points_params->actsubsch = num_cycles - 1;
for (port = 0; port < SJA1105_NUM_PORTS; port++) {
const struct tc_taprio_qopt_offload *tas_config;
+ /* Relative base time */
+ s64 rbt;
tas_config = tas_data->config[port];
if (!tas_config)
@@ -193,13 +300,20 @@ static int sja1105_init_scheduling(struct sja1105_private *priv)
schedule_start_idx = k;
schedule_end_idx = k + tas_config->num_entries - 1;
- /* TODO this is only a relative base time for the subschedule
- * (relative to PTPSCHTM). But as we're using standalone and
- * not PTP clock as time reference, leave it like this for now.
- * Later we'll have to enforce that all ports' base times are
- * within SJA1105_TAS_MAX_DELTA 200ns cycles of one another.
+ /* This is only a relative base time for the subschedule
+ * (relative to PTPSCHTM - aka the operational base time).
*/
- entry_point_delta = ns_to_sja1105_delta(tas_config->base_time);
+ rbt = future_base_time(tas_config->base_time,
+ tas_config->cycle_time,
+ tas_data->earliest_base_time);
+ rbt -= tas_data->earliest_base_time;
+ /* UM10944.pdf 4.2.2. Schedule Entry Points table says that
+ * delta cannot be zero, which is shitty. Advance all relative
+ * base times by 1 TAS delta, so that even the earliest base
+ * time becomes 1 in relative terms. Then start the operational
+ * base time (PTPSCHTM) one TAS delta earlier than planned.
+ */
+ entry_point_delta = ns_to_sja1105_delta(rbt) + 1;
schedule_entry_points[cycle].subschindx = cycle;
schedule_entry_points[cycle].delta = entry_point_delta;
@@ -405,8 +519,302 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
return sja1105_static_config_reload(priv);
}
+static int sja1105_tas_check_running(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+ struct sja1105_ptp_cmd cmd = {0};
+ int rc;
+
+ rc = sja1105_ptp_commit(priv, &cmd, SPI_READ);
+ if (rc < 0)
+ return rc;
+
+ if (cmd.ptpstrtsch == 1)
+ /* Schedule successfully started */
+ tas_data->state = SJA1105_TAS_STATE_RUNNING;
+ else if (cmd.ptpstopsch == 1)
+ /* Schedule is stopped */
+ tas_data->state = SJA1105_TAS_STATE_DISABLED;
+ else
+ /* Schedule is probably not configured with PTP clock source */
+ rc = -EINVAL;
+
+ return rc;
+}
+
+/* Write to PTPCLKCORP */
+static int sja1105_tas_adjust_drift(struct sja1105_private *priv,
+ u64 correction)
+{
+ const struct sja1105_regs *regs = priv->info->regs;
+ u64 ptpclkcorp = ns_to_sja1105_ticks(correction);
+
+ return sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkcorp,
+ &ptpclkcorp, 4, NULL);
+}
+
+/* Write to PTPSCHTM */
+static int sja1105_tas_set_base_time(struct sja1105_private *priv,
+ u64 base_time)
+{
+ const struct sja1105_regs *regs = priv->info->regs;
+ u64 ptpschtm = ns_to_sja1105_ticks(base_time);
+
+ return sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpschtm,
+ &ptpschtm, 8, NULL);
+}
+
+static int sja1105_tas_start(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+ struct sja1105_ptp_cmd *cmd = &priv->ptp_data.cmd;
+ int rc;
+
+ dev_dbg(priv->ds->dev, "Starting the TAS\n");
+
+ if (tas_data->state == SJA1105_TAS_STATE_ENABLED_NOT_RUNNING ||
+ tas_data->state == SJA1105_TAS_STATE_RUNNING) {
+ dev_err(priv->ds->dev, "TAS already started\n");
+ return -EINVAL;
+ }
+
+ cmd->ptpstrtsch = 1;
+ cmd->ptpstopsch = 0;
+
+ rc = sja1105_ptp_commit(priv, cmd, SPI_WRITE);
+ if (rc < 0)
+ return rc;
+
+ tas_data->state = SJA1105_TAS_STATE_ENABLED_NOT_RUNNING;
+
+ return 0;
+}
+
+static int sja1105_tas_stop(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+ struct sja1105_ptp_cmd *cmd = &priv->ptp_data.cmd;
+ int rc;
+
+ dev_dbg(priv->ds->dev, "Stopping the TAS\n");
+
+ if (tas_data->state == SJA1105_TAS_STATE_DISABLED) {
+ dev_err(priv->ds->dev, "TAS already disabled\n");
+ return -EINVAL;
+ }
+
+ cmd->ptpstopsch = 1;
+ cmd->ptpstrtsch = 0;
+
+ rc = sja1105_ptp_commit(priv, cmd, SPI_WRITE);
+ if (rc < 0)
+ return rc;
+
+ tas_data->state = SJA1105_TAS_STATE_DISABLED;
+
+ return 0;
+}
+
+/* The schedule engine and the PTP clock are driven by the same oscillator, and
+ * they run in parallel. But whilst the PTP clock can keep an absolute
+ * time-of-day, the schedule engine is only running in 'ticks' (25 ticks make
+ * up a delta, which is 200ns), and wrapping around at the end of each cycle.
+ * The schedule engine is started when the PTP clock reaches the PTPSCHTM time
+ * (in PTP domain).
+ * Because the PTP clock can be rate-corrected (accelerated or slowed down) by
+ * a software servo, and the schedule engine clock runs in parallel to the PTP
+ * clock, there is logic internal to the switch that periodically keeps the
+ * schedule engine from drifting away. The frequency with which this internal
+ * syntonization happens is the PTP clock correction period (PTPCLKCORP). It is
+ * a value also in the PTP clock domain, and is also rate-corrected.
+ * To be precise, during a correction period, there is logic to determine by
+ * how many scheduler clock ticks has the PTP clock drifted. At the end of each
+ * correction period/beginning of new one, the length of a delta is shrunk or
+ * expanded with an integer number of ticks, compared with the typical 25.
+ * So a delta lasts for 200ns (or 25 ticks) only on average.
+ * Sometimes it is longer, sometimes it is shorter. The internal syntonization
+ * logic can adjust for at most 5 ticks each 20 ticks.
+ *
+ * The first implication is that you should choose your schedule correction
+ * period to be an integer multiple of the schedule length. Preferably one.
+ * In case there are schedules of multiple ports active, then the correction
+ * period needs to be a multiple of them all. Given the restriction that the
+ * cycle times have to be multiples of one another anyway, this means the
+ * correction period can simply be the largest cycle time, hence the current
+ * choice. This way, the updates are always synchronous to the transmission
+ * cycle, and therefore predictable.
+ *
+ * The second implication is that at the beginning of a correction period, the
+ * first few deltas will be modulated in time, until the schedule engine is
+ * properly phase-aligned with the PTP clock. For this reason, you should place
+ * your best-effort traffic at the beginning of a cycle, and your
+ * time-triggered traffic afterwards.
+ *
+ * The third implication is that once the schedule engine is started, it can
+ * only adjust for so much drift within a correction period. In the servo you
+ * can only change the PTPCLKRATE, but not step the clock (PTPCLKADD). If you
+ * want to do the latter, you need to stop and restart the schedule engine,
+ * which is what the state machine handles.
+ */
+static void sja1105_tas_state_machine(struct work_struct *work)
+{
+ struct sja1105_tas_data *tas_data = work_to_sja1105_tas(work);
+ struct sja1105_private *priv = tas_to_sja1105(tas_data);
+ struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
+ struct timespec64 base_time_ts, now_ts;
+ struct dsa_switch *ds = priv->ds;
+ struct timespec64 diff;
+ s64 base_time, now;
+ int rc = 0;
+
+ mutex_lock(&ptp_data->lock);
+
+ switch (tas_data->state) {
+ case SJA1105_TAS_STATE_DISABLED:
+
+ dev_dbg(ds->dev, "TAS state: disabled\n");
+ /* Can't do anything at all if clock is still being stepped */
+ if (tas_data->last_op != SJA1105_PTP_ADJUSTFREQ)
+ break;
+
+ rc = sja1105_tas_adjust_drift(priv, tas_data->max_cycle_time);
+ if (rc < 0)
+ break;
+
+ now = __sja1105_ptp_gettimex(priv, NULL);
+
+ /* Plan to start the earliest schedule first. The others
+ * will be started in hardware, by way of their respective
+ * entry points delta.
+ * Try our best to avoid fringe cases (race condition between
+ * ptpschtm and ptpstrtsch) by pushing the oper_base_time at
+ * least one second in the future from now. This is not ideal,
+ * but this only needs to buy us time until the
+ * sja1105_tas_start command below gets executed.
+ */
+ base_time = future_base_time(tas_data->earliest_base_time,
+ tas_data->max_cycle_time,
+ now + 1ull * NSEC_PER_SEC);
+ base_time -= sja1105_delta_to_ns(1);
+
+ rc = sja1105_tas_set_base_time(priv, base_time);
+ if (rc < 0)
+ break;
+
+ tas_data->oper_base_time = base_time;
+
+ rc = sja1105_tas_start(priv);
+ if (rc < 0)
+ break;
+
+ base_time_ts = ns_to_timespec64(base_time);
+ now_ts = ns_to_timespec64(now);
+
+ dev_dbg(ds->dev, "OPER base time %lld.%09ld (now %lld.%09ld)\n",
+ base_time_ts.tv_sec, base_time_ts.tv_nsec,
+ now_ts.tv_sec, now_ts.tv_nsec);
+
+ break;
+
+ case SJA1105_TAS_STATE_ENABLED_NOT_RUNNING:
+ /* Check if TAS has actually started, by comparing the
+ * scheduled start time with the SJA1105 PTP clock
+ */
+ dev_dbg(ds->dev, "TAS state: enabled but not running\n");
+
+ /* Clock was stepped.. bad news for TAS */
+ if (tas_data->last_op != SJA1105_PTP_ADJUSTFREQ) {
+ sja1105_tas_stop(priv);
+ break;
+ }
+
+ now = __sja1105_ptp_gettimex(priv, NULL);
+
+ if (now < tas_data->oper_base_time) {
+ /* TAS has not started yet */
+ diff = ns_to_timespec64(tas_data->oper_base_time - now);
+ dev_dbg(ds->dev, "time to start: [%lld.%09ld]",
+ diff.tv_sec, diff.tv_nsec);
+ break;
+ }
+
+ /* Time elapsed, what happened? */
+ rc = sja1105_tas_check_running(priv);
+ if (rc < 0)
+ break;
+
+ if (tas_data->state == SJA1105_TAS_STATE_RUNNING)
+ /* TAS has started */
+ dev_dbg(ds->dev, "TAS state: transitioned to running\n");
+ else
+ dev_err(ds->dev, "TAS state: not started despite time elapsed\n");
+
+ break;
+
+ case SJA1105_TAS_STATE_RUNNING:
+ dev_dbg(ds->dev, "TAS state: running\n");
+
+ /* Clock was stepped.. bad news for TAS */
+ if (tas_data->last_op != SJA1105_PTP_ADJUSTFREQ) {
+ sja1105_tas_stop(priv);
+ break;
+ }
+
+ rc = sja1105_tas_check_running(priv);
+ if (rc < 0)
+ break;
+
+ if (tas_data->state != SJA1105_TAS_STATE_RUNNING) {
+ dev_err(ds->dev, "TAS surprisingly stopped\n");
+ break;
+ }
+
+ now = __sja1105_ptp_gettimex(priv, NULL);
+
+ diff = ns_to_timespec64(now - tas_data->oper_base_time);
+
+ dev_dbg(ds->dev, "Time since TAS started: [%lld.%09ld]\n",
+ diff.tv_sec, diff.tv_nsec);
+ break;
+
+ default:
+ if (net_ratelimit())
+ dev_err(ds->dev, "TAS in an invalid state (incorrect use of API)!\n");
+ }
+
+ if (rc && net_ratelimit())
+ dev_err(ds->dev, "An operation returned %d\n", rc);
+
+ mutex_unlock(&ptp_data->lock);
+}
+
+void sja1105_tas_clockstep(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+
+ if (!tas_data->enabled)
+ return;
+
+ tas_data->last_op = SJA1105_PTP_CLOCKSTEP;
+ schedule_work(&tas_data->tas_work);
+}
+
+void sja1105_tas_adjfreq(struct sja1105_private *priv)
+{
+ struct sja1105_tas_data *tas_data = &priv->tas_data;
+
+ if (!tas_data->enabled)
+ return;
+
+ tas_data->last_op = SJA1105_PTP_ADJUSTFREQ;
+ schedule_work(&tas_data->tas_work);
+}
+
void sja1105_tas_setup(struct sja1105_private *priv)
{
+ INIT_WORK(&priv->tas_data.tas_work, sja1105_tas_state_machine);
+ priv->tas_data.state = SJA1105_TAS_STATE_DISABLED;
+ priv->tas_data.last_op = SJA1105_PTP_NONE;
}
void sja1105_tas_teardown(struct sja1105_private *priv)
@@ -414,6 +822,8 @@ void sja1105_tas_teardown(struct sja1105_private *priv)
struct sja1105_tas_data *tas_data = &priv->tas_data;
int port;
+ cancel_work_sync(&tas_data->tas_work);
+
for (port = 0; port < SJA1105_NUM_PORTS; port++)
if (tas_data->config[port])
taprio_free(tas_data->config[port]);
diff --git a/drivers/net/dsa/sja1105/sja1105_tas.h b/drivers/net/dsa/sja1105/sja1105_tas.h
index 0ef82810d9d7..ecc95624e3f6 100644
--- a/drivers/net/dsa/sja1105/sja1105_tas.h
+++ b/drivers/net/dsa/sja1105/sja1105_tas.h
@@ -8,8 +8,27 @@
#if IS_ENABLED(CONFIG_NET_DSA_SJA1105_TAS)
+enum sja1105_tas_state {
+ SJA1105_TAS_STATE_DISABLED,
+ SJA1105_TAS_STATE_ENABLED_NOT_RUNNING,
+ SJA1105_TAS_STATE_RUNNING,
+};
+
+enum sja1105_ptp_op {
+ SJA1105_PTP_NONE,
+ SJA1105_PTP_CLOCKSTEP,
+ SJA1105_PTP_ADJUSTFREQ,
+};
+
struct sja1105_tas_data {
struct tc_taprio_qopt_offload *config[SJA1105_NUM_PORTS];
+ enum sja1105_tas_state state;
+ enum sja1105_ptp_op last_op;
+ struct work_struct tas_work;
+ s64 earliest_base_time;
+ s64 oper_base_time;
+ u64 max_cycle_time;
+ bool enabled;
};
int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
@@ -19,6 +38,10 @@ void sja1105_tas_setup(struct sja1105_private *priv);
void sja1105_tas_teardown(struct sja1105_private *priv);
+void sja1105_tas_clockstep(struct sja1105_private *priv);
+
+void sja1105_tas_adjfreq(struct sja1105_private *priv);
+
#else
/* C doesn't allow empty structures, bah! */
@@ -37,6 +60,10 @@ static inline void sja1105_tas_setup(struct sja1105_private *priv) { }
static inline void sja1105_tas_teardown(struct sja1105_private *priv) { }
+static inline void sja1105_tas_clockstep(struct sja1105_private *priv) { }
+
+static inline void sja1105_tas_adjfreq(struct sja1105_private *priv) { }
+
#endif /* IS_ENABLED(CONFIG_NET_DSA_SJA1105_TAS) */
#endif /* _SJA1105_TAS_H */
--
2.17.1
^ permalink raw reply related
* [PATCH v1 net-next 08/15] net: dsa: sja1105: Advertise the 8 TX queues
From: Vladimir Oltean @ 2019-09-02 16:25 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem, vinicius.gomes,
vedang.patel, richardcochran
Cc: weifeng.voon, jiri, m-karicheri2, Jose.Abreu, ilias.apalodimas,
jhs, xiyou.wangcong, kurt.kanzenbach, netdev, Vladimir Oltean
In-Reply-To: <20190902162544.24613-1-olteanv@gmail.com>
This is a preparation patch for the tc-taprio offload (and potentially
for other future offloads such as tc-mqprio).
Instead of looking directly at skb->priority during xmit, let's get the
netdev queue and the queue-to-traffic-class mapping, and put the
resulting traffic class into the dsa_8021q PCP field. The switch is
configured with a 1-to-1 PCP-to-ingress-queue-to-egress-queue mapping
(see vlan_pmap in sja1105_main.c), so the effect is that we can inject
into a front-panel's egress traffic class through VLAN tagging from
Linux, completely transparently.
Unfortunately the switch doesn't look at the VLAN PCP in the case of
management traffic to/from the CPU (link-local frames at
01-80-C2-xx-xx-xx or 01-1B-19-xx-xx-xx) so we can't alter the
transmission queue of this type of traffic on a frame-by-frame basis. It
is only selected through the "hostprio" setting which ATM is harcoded in
the driver to 7.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
Changes since RFC:
- None.
drivers/net/dsa/sja1105/sja1105_main.c | 7 ++++++-
net/dsa/tag_sja1105.c | 3 ++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 670c069722d5..8b930cc2dabc 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -384,7 +384,9 @@ static int sja1105_init_general_params(struct sja1105_private *priv)
/* Disallow dynamic changing of the mirror port */
.mirr_ptacu = 0,
.switchid = priv->ds->index,
- /* Priority queue for link-local frames trapped to CPU */
+ /* Priority queue for link-local management frames
+ * (both ingress to and egress from CPU - PTP, STP etc)
+ */
.hostprio = 7,
.mac_fltres1 = SJA1105_LINKLOCAL_FILTER_A,
.mac_flt1 = SJA1105_LINKLOCAL_FILTER_A_MASK,
@@ -1745,6 +1747,9 @@ static int sja1105_setup(struct dsa_switch *ds)
*/
ds->vlan_filtering_is_global = true;
+ /* Advertise the 8 egress queues */
+ ds->num_tx_queues = SJA1105_NUM_TC;
+
/* The DSA/switchdev model brings up switch ports in standalone mode by
* default, and that means vlan_filtering is 0 since they're not under
* a bridge, so it's safe to set up switch tagging at this time.
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 47ee88163a9d..9c9aff3e52cf 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -89,7 +89,8 @@ static struct sk_buff *sja1105_xmit(struct sk_buff *skb,
struct dsa_port *dp = dsa_slave_to_port(netdev);
struct dsa_switch *ds = dp->ds;
u16 tx_vid = dsa_8021q_tx_vid(ds, dp->index);
- u8 pcp = skb->priority;
+ u16 queue_mapping = skb_get_queue_mapping(skb);
+ u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
/* Transmitting management traffic does not rely upon switch tagging,
* but instead SPI-installed management routes. Part 2 of this
--
2.17.1
^ permalink raw reply related
* Re: [GIT] Networking
From: pr-tracker-bot @ 2019-09-02 17:05 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20190901.134525.286041997131171719.davem@davemloft.net>
The pull request you sent on Sun, 01 Sep 2019 13:45:25 -0700 (PDT):
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/345464fb760d1b772e891538b498e111c588b692
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [net-next 1/3] ravb: correct typo in FBP field of SFO register
From: Sergei Shtylyov @ 2019-09-02 17:12 UTC (permalink / raw)
To: Simon Horman, David Miller
Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi
In-Reply-To: <20190902080603.5636-2-horms+renesas@verge.net.au>
Hello!
On 09/02/2019 11:06 AM, Simon Horman wrote:
> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> The field name is FBP rather than FPB.
>
> This field is unused and could equally be removed from the driver entirely.
> But there seems no harm in leaving as documentation of the presence of the
> field.
>
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> ---
> v0 - Kazuya Mizuguchi
>
> v1 - Simon Horman
> * Extracted from larger patch
I'd just claim the authorship in this case (and mentioned that it's based
on Mizuguchi-san's large patch right in the change log).
> * Wrote changelog
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index ac9195add811..bdb051f04b0c 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -317,7 +312,7 @@ enum UFCD_BIT {
>
> /* SFO */
> enum SFO_BIT {
> - SFO_FPB = 0x0000003F,
> + SFO_FBP = 0x0000003F,
> };
>
> /* RTC */
> ---
This is where the actual patch starts, right?
> drivers/net/ethernet/renesas/ravb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index ac9195add811..2596a95a4300 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -317,7 +317,7 @@ enum UFCD_BIT {
>
> /* SFO */
> enum SFO_BIT {
> - SFO_FPB = 0x0000003F,
> + SFO_FBP = 0x0000003F,
> };
>
> /* RTC */
MBR, Sergei
^ permalink raw reply
* Re: [net-next 11/15] i40e: Implement debug macro hw_dbg using pr_debug
From: Mauro Rodrigues @ 2019-09-02 17:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jeff Kirsher, davem, netdev, nhorman, sassmann, Andrew Bowers
In-Reply-To: <20190828153936.57ababbc@cakuba.netronome.com>
On Wed, Aug 28, 2019 at 03:39:53PM -0700, Jakub Kicinski wrote:
> On Tue, 27 Aug 2019 23:44:03 -0700, Jeff Kirsher wrote:
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_osdep.h b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
> > index a07574bff550..c0c9ce3eab23 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_osdep.h
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_osdep.h
> > @@ -18,7 +18,12 @@
> > * actual OS primitives
> > */
> >
> > -#define hw_dbg(hw, S, A...) do {} while (0)
> > +#define hw_dbg(hw, S, A...) \
> > +do { \
> > + int domain = pci_domain_nr(((struct i40e_pf *)(hw)->back)->pdev->bus); \
> > + pr_debug("i40e %04x:%02x:%02x.%x " S, domain, (hw)->bus.bus_id, \
> > + (hw)->bus.device, (hw)->bus.func, ## A); \
>
> This looks like open coded dev_dbg() / dev_namie(), why?
Indeed, thanks for pointing out. I'll fix this and the other patch you
reviewed and resubmit.
I'm not sure what should be the preferred approach here though, just use
dev_dbg to implement this macro or replace all of its occurrence in the
code by dev_dbg?
Jeff, do you have any preference?
>
> > +} while (0)
^ permalink raw reply
* Re: [net-next 2/3] ravb: Remove undocumented processing
From: Sergei Shtylyov @ 2019-09-02 17:41 UTC (permalink / raw)
To: Simon Horman, David Miller
Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi
In-Reply-To: <20190902080603.5636-3-horms+renesas@verge.net.au>
On 09/02/2019 11:06 AM, Simon Horman wrote:
> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> This patch removes the use of the undocumented registers
> CDCR, LCCR, CERCR, CEECR and the undocumented BOC bit of the CCC register.
The driver has many more #define's marked as undocumented. It's not clear
why you crammed the counters and the endianness bit in one patch. It clearly
needs to be split -- one patch for the MAC counters and one patch for the
AVB-DMAC bit.
> Current documentation for EtherAVB (ravb) describes the offset of
> what the driver uses as the BOC bit as reserved and that only a value of
> 0 should be written. Furthermore, the offsets used for the undocumented
> registers are also considered reserved nd should not be written to.
>
> After some internal investigation with Renesas it remains unclear
> why this driver accesses these fields but regardless of what the historical
> reasons are the current code is considered incorrect.
>
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Allan W. Nielsen @ 2019-09-02 17:42 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830063624.GN2312@nanopsycho>
Hi Jiri,
Sorry for joining the discussion this late, but I have been without mail access
for the last few days.
The 08/30/2019 08:36, Jiri Pirko wrote:
> Fri, Aug 30, 2019 at 08:02:33AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Fri, 30 Aug 2019 07:39:40 +0200
> >
> >> Because the "promisc mode" would gain another meaning. Now how the
> >> driver should guess which meaning the user ment when he setted it?
> >> filter or trap?
> >>
> >> That is very confusing. If the flag is the way to do this, let's
> >> introduce another flag, like IFF_TRAPPING indicating that user wants
> >> exactly this.
> >
> >I don't understand how the meaning of promiscuous mode for a
> >networking device has suddenly become ambiguous, when did this start
> >happening?
>
> The promiscuity is a way to setup the rx filter. So promics == rx filter
> off. For normal nics, where there is no hw fwd datapath,
> this coincidentally means all received packets go to cpu.
> But if there is hw fwd datapath, rx filter is still off, all rxed packets
> are processed. But that does not mean they should be trapped to cpu.
>
> Simple example:
> I need to see slowpath packets, for example arps/stp/bgp/... that
> are going to cpu, I do:
> tcpdump -i swp1
How is this different from "tcpdump -p -i swp1"
> I don't want to get all the traffic running over hw running this cmd.
> This is a valid usecase.
>
> To cope with hw fwd datapath devices, I believe that tcpdump has to have
> notion of that. Something like:
>
> tcpdump -i swp1 --hw-trapping-mode
>
> The logic can be inverse:
> tcpdump -i swp1
> tcpdump -i swp1 --no-hw-trapping-mode
>
> However, that would provide inconsistent behaviour between existing and
> patched tcpdump/kernel.
>
> All I'm trying to say, there are 2 flags
> needed (if we don't use tc trap).
I have been reading through this thread several times and I still do not get it.
As far as I understand you are arguing that we need 3 modes:
- tcpdump -i swp1
- tcpdump -p -i swp1
- tcpdump -i swp1 --hw-trapping-mode
Would you mind provide an example of the traffic you want to see in the 3 cases
(or the traffic which you do not want to see).
/Allan
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Jiri Pirko @ 2019-09-02 17:51 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: David Miller, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190902174229.uur7r7duq4dvbnqq@lx-anielsen.microsemi.net>
Mon, Sep 02, 2019 at 07:42:31PM CEST, allan.nielsen@microchip.com wrote:
>Hi Jiri,
>
>Sorry for joining the discussion this late, but I have been without mail access
>for the last few days.
>
>
>The 08/30/2019 08:36, Jiri Pirko wrote:
>> Fri, Aug 30, 2019 at 08:02:33AM CEST, davem@davemloft.net wrote:
>> >From: Jiri Pirko <jiri@resnulli.us>
>> >Date: Fri, 30 Aug 2019 07:39:40 +0200
>> >
>> >> Because the "promisc mode" would gain another meaning. Now how the
>> >> driver should guess which meaning the user ment when he setted it?
>> >> filter or trap?
>> >>
>> >> That is very confusing. If the flag is the way to do this, let's
>> >> introduce another flag, like IFF_TRAPPING indicating that user wants
>> >> exactly this.
>> >
>> >I don't understand how the meaning of promiscuous mode for a
>> >networking device has suddenly become ambiguous, when did this start
>> >happening?
>>
>> The promiscuity is a way to setup the rx filter. So promics == rx filter
>> off. For normal nics, where there is no hw fwd datapath,
>> this coincidentally means all received packets go to cpu.
>> But if there is hw fwd datapath, rx filter is still off, all rxed packets
>> are processed. But that does not mean they should be trapped to cpu.
>>
>> Simple example:
>> I need to see slowpath packets, for example arps/stp/bgp/... that
>> are going to cpu, I do:
>> tcpdump -i swp1
>
>How is this different from "tcpdump -p -i swp1"
>
>> I don't want to get all the traffic running over hw running this cmd.
>> This is a valid usecase.
>>
>> To cope with hw fwd datapath devices, I believe that tcpdump has to have
>> notion of that. Something like:
>>
>> tcpdump -i swp1 --hw-trapping-mode
>>
>> The logic can be inverse:
>> tcpdump -i swp1
>> tcpdump -i swp1 --no-hw-trapping-mode
>>
>> However, that would provide inconsistent behaviour between existing and
>> patched tcpdump/kernel.
>>
>> All I'm trying to say, there are 2 flags
>> needed (if we don't use tc trap).
>
>I have been reading through this thread several times and I still do not get it.
>
>As far as I understand you are arguing that we need 3 modes:
>
>- tcpdump -i swp1
Depends on default. Promisc is on.
>- tcpdump -p -i swp1
All traffic that is trapped to the cpu by default, not promisc means
only mac of the interface (if bridge for example haven't set promisc
already) and special macs. So host traffic (ip of host), bgp, arp, nsnd,
etc.
>- tcpdump -i swp1 --hw-trapping-mode
Promisc is on, all traffic received on the port and pushed to cpu. User
has to be careful because in case of mlxsw this can lead to couple
hundred gigabit traffic going over limited pci bandwidth (gigabits).
>
>Would you mind provide an example of the traffic you want to see in the 3 cases
>(or the traffic which you do not want to see).
>
>/Allan
>
^ permalink raw reply
* Re: [net-next 3/3] ravb: TROCR register is only present on R-Car Gen3
From: Sergei Shtylyov @ 2019-09-02 17:54 UTC (permalink / raw)
To: Simon Horman, David Miller; +Cc: Magnus Damm, netdev, linux-renesas-soc
In-Reply-To: <20190902080603.5636-4-horms+renesas@verge.net.au>
On 09/02/2019 11:06 AM, Simon Horman wrote:
> Only use the TROCR register on R-Car Gen3.
> It is not present on other SoCs.
>
> Offsets used for the undocumented registers are also considered reserved
> and should not be written to.
>
> After some internal investigation with Renesas it remains unclear why this
> driver accesses these fields on R-Car Gen2 but regardless of what the
> historical reasons are the current code is considered incorrect.
Most probably copy&paste from sh_eth.c...
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
[...]
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Allan W. Nielsen @ 2019-09-02 17:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ido Schimmel, David Miller, jiri, horatiu.vultur,
alexandre.belloni, UNGLinuxDriver, ivecera, f.fainelli, netdev,
linux-kernel
In-Reply-To: <20190901184819.GA24673@lunn.ch>
The 09/01/2019 20:48, Andrew Lunn wrote:
> > Look, this again boils down to what promisc mode means with regards to
> > hardware offload. You want it to mean punt all traffic to the CPU? Fine.
> > Does not seem like anyone will be switching sides anyway, so lets move
> > forward. But the current approach is not good. Each driver needs to have
> > this special case logic and the semantics of promisc mode change not
> > only with regards to the value of the promisc counter, but also with
> > regards to the interface's uppers. This is highly fragile and confusing.
> Yes, i agree. We want one function, in the core, which handles all the
> different uppers. Maybe 2, if we need to consider L2 and L3 switches
> differently.
Are you suggesting that we continue the path in v3, but add a utility function
to determinate if the interface needs to go into promisc mode (taking the
bridge stats, the promisc counter, and the upper devices into account).
Or are you suggest that we like Ido go back to v2?
/Allan
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Allan W. Nielsen @ 2019-09-02 18:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190902175124.GA2312@nanopsycho>
The 09/02/2019 19:51, Jiri Pirko wrote:
> External E-Mail
>
>
> Mon, Sep 02, 2019 at 07:42:31PM CEST, allan.nielsen@microchip.com wrote:
> >Hi Jiri,
> >
> >Sorry for joining the discussion this late, but I have been without mail access
> >for the last few days.
> >
> >
> >The 08/30/2019 08:36, Jiri Pirko wrote:
> >> Fri, Aug 30, 2019 at 08:02:33AM CEST, davem@davemloft.net wrote:
> >> >From: Jiri Pirko <jiri@resnulli.us>
> >> >Date: Fri, 30 Aug 2019 07:39:40 +0200
> >> >
> >> >> Because the "promisc mode" would gain another meaning. Now how the
> >> >> driver should guess which meaning the user ment when he setted it?
> >> >> filter or trap?
> >> >>
> >> >> That is very confusing. If the flag is the way to do this, let's
> >> >> introduce another flag, like IFF_TRAPPING indicating that user wants
> >> >> exactly this.
> >> >
> >> >I don't understand how the meaning of promiscuous mode for a
> >> >networking device has suddenly become ambiguous, when did this start
> >> >happening?
> >>
> >> The promiscuity is a way to setup the rx filter. So promics == rx filter
> >> off. For normal nics, where there is no hw fwd datapath,
> >> this coincidentally means all received packets go to cpu.
> >> But if there is hw fwd datapath, rx filter is still off, all rxed packets
> >> are processed. But that does not mean they should be trapped to cpu.
> >>
> >> Simple example:
> >> I need to see slowpath packets, for example arps/stp/bgp/... that
> >> are going to cpu, I do:
> >> tcpdump -i swp1
> >
> >How is this different from "tcpdump -p -i swp1"
> >
> >> I don't want to get all the traffic running over hw running this cmd.
> >> This is a valid usecase.
> >>
> >> To cope with hw fwd datapath devices, I believe that tcpdump has to have
> >> notion of that. Something like:
> >>
> >> tcpdump -i swp1 --hw-trapping-mode
> >>
> >> The logic can be inverse:
> >> tcpdump -i swp1
> >> tcpdump -i swp1 --no-hw-trapping-mode
> >>
> >> However, that would provide inconsistent behaviour between existing and
> >> patched tcpdump/kernel.
> >>
> >> All I'm trying to say, there are 2 flags
> >> needed (if we don't use tc trap).
> >
> >I have been reading through this thread several times and I still do not get it.
> >
> >As far as I understand you are arguing that we need 3 modes:
> >
> >- tcpdump -i swp1
>
> Depends on default. Promisc is on.
>
>
> >- tcpdump -p -i swp1
>
> All traffic that is trapped to the cpu by default, not promisc means
> only mac of the interface (if bridge for example haven't set promisc
> already) and special macs. So host traffic (ip of host), bgp, arp, nsnd,
> etc.
In the case where the interface is enslaved to a bridge, it is put into promisc
mode, which means that "tcpdump -i swp1" and "tcpdump -p -i swp1" give the same
result, right?
Is this desirable?
> >- tcpdump -i swp1 --hw-trapping-mode
>
> Promisc is on, all traffic received on the port and pushed to cpu. User
> has to be careful because in case of mlxsw this can lead to couple
> hundred gigabit traffic going over limited pci bandwidth (gigabits).
>
>
> >
> >Would you mind provide an example of the traffic you want to see in the 3 cases
> >(or the traffic which you do not want to see).
> >
> >/Allan
> >
>
--
/Allan
^ permalink raw reply
* [PATCH v3 1/2] net: bridge: use mac_len in bridge forwarding
From: Zahari Doychev @ 2019-09-02 18:09 UTC (permalink / raw)
To: netdev
Cc: bridge, nikolay, roopa, jhs, dsahern, simon.horman,
makita.toshiaki, xiyou.wangcong, jiri, alexei.starovoitov,
johannes, Zahari Doychev
The bridge code cannot forward packets from various paths that set up the
SKBs in different ways. Some of these packets get corrupted during the
forwarding as not always is just ETH_HLEN pulled at the front.
This happens e.g. when VLAN tags are pushed by using tc act_vlan on
ingress. Example configuration is provided below. The test setup consists
of two netdevs connected to external hosts. There is act_vlan on one of
them adding two vlan tags on ingress and removing the tags on egress.
The configuration is done using the following commands:
ip link add name br0 type bridge vlan_filtering 1
ip link set dev br0 up
ip link set dev net0 up
ip link set dev net0 master br0
ip link set dev net1 up
ip link set dev net1 master br0
bridge vlan add dev net0 vid 100 master
bridge vlan add dev br0 vid 100 self
bridge vlan add dev net1 vid 100 master
tc qdisc add dev net0 handle ffff: clsact
tc qdisc add dev net1 handle ffff: clsact
tc filter add dev net0 ingress pref 1 protocol all flower \
action vlan push id 10 pipe action vlan push id 100
tc filter add dev net0 egress pref 1 protocol 802.1q flower \
vlan_id 100 vlan_ethtype 802.1q cvlan_id 10 \
action vlan pop pipe action vlan pop
When using the setup above the packets coming on net0 get double tagged but
the MAC headers gets corrupted when the packets go out of net1.
The skb->data is pushed only by the ETH_HLEN length instead of mac_len in
br_dev_queue_push_xmit. This later causes the function validate_xmit_vlan
to insert the outer vlan tag behind the inner vlan tag as the skb->data
does not point to the start of packet.
The problem is fixed by using skb->mac_len instead of ETH_HLEN, which makes
sure that the skb headers are correctly restored. This usually does not
change anything, execpt the local bridge transmits which now need to set
the skb->mac_len correctly in br_dev_xmit, as well as the broken case noted
above.
Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
v2->v3:
- move cover letter description to commit message
---
net/bridge/br_device.c | 3 ++-
net/bridge/br_forward.c | 4 ++--
net/bridge/br_vlan.c | 3 ++-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 681b72862c16..aeb77ff60311 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -55,8 +55,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
BR_INPUT_SKB_CB(skb)->frag_max_size = 0;
skb_reset_mac_header(skb);
+ skb_reset_mac_len(skb);
eth = eth_hdr(skb);
- skb_pull(skb, ETH_HLEN);
+ skb_pull(skb, skb->mac_len);
if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &vid))
goto out;
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 86637000f275..edb4f3533f05 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -32,7 +32,7 @@ static inline int should_deliver(const struct net_bridge_port *p,
int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
{
- skb_push(skb, ETH_HLEN);
+ skb_push(skb, skb->mac_len);
if (!is_skb_forwardable(skb->dev, skb))
goto drop;
@@ -94,7 +94,7 @@ static void __br_forward(const struct net_bridge_port *to,
net = dev_net(indev);
} else {
if (unlikely(netpoll_tx_running(to->br->dev))) {
- skb_push(skb, ETH_HLEN);
+ skb_push(skb, skb->mac_len);
if (!is_skb_forwardable(skb->dev, skb))
kfree_skb(skb);
else
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index bb98984cd27d..419067b314d7 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -466,13 +466,14 @@ static bool __allowed_ingress(const struct net_bridge *br,
/* Tagged frame */
if (skb->vlan_proto != br->vlan_proto) {
/* Protocol-mismatch, empty out vlan_tci for new tag */
- skb_push(skb, ETH_HLEN);
+ skb_push(skb, skb->mac_len);
skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
skb_vlan_tag_get(skb));
if (unlikely(!skb))
return false;
skb_pull(skb, ETH_HLEN);
+ skb_reset_network_header(skb);
skb_reset_mac_len(skb);
*vid = 0;
tagged = false;
--
2.22.0
^ permalink raw reply related
* [PATCH v3 2/2] selftests: forwrading: tc vlan bridge test
From: Zahari Doychev @ 2019-09-02 18:10 UTC (permalink / raw)
To: netdev
Cc: bridge, nikolay, roopa, jhs, dsahern, simon.horman,
makita.toshiaki, xiyou.wangcong, jiri, alexei.starovoitov,
johannes, Zahari Doychev
In-Reply-To: <20190902181000.25638-1-zahari.doychev@linux.com>
Add bridge vlan aware forwarding test for vlans added by tc-act_vlan. The
forwarding is tested in two cases when the bridge protocol and outer vlan
tag protocol match and mismatch. The tests checks the correct usage of
skb->mac_len in the bridge code.
Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
v2->v3:
- selftest added
---
.../forwarding/bridge_vlan_aware_tc_vlan.sh | 187 ++++++++++++++++++
1 file changed, 187 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/bridge_vlan_aware_tc_vlan.sh
diff --git a/tools/testing/selftests/net/forwarding/bridge_vlan_aware_tc_vlan.sh b/tools/testing/selftests/net/forwarding/bridge_vlan_aware_tc_vlan.sh
new file mode 100755
index 000000000000..215d6293fa54
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/bridge_vlan_aware_tc_vlan.sh
@@ -0,0 +1,187 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# This test uses the standard topology for testing bridge forwarding. See
+# README for more details.
+#
+# tc vlan actions are applied on one of the bridge ports and on other one
+# the corresponding vlan network devices are created.
+#
+
+ALL_TESTS="
+ test_tc_vlan_bridge_ipv4_forwarding
+ test_tc_vlan_bridge_ipv4_forwarding_proto
+ test_tc_vlan_bridge_ipv6_forwarding
+ test_tc_vlan_bridge_ipv6_forwarding_proto
+"
+
+NUM_NETIFS=4
+CHECK_TC="yes"
+source lib.sh
+
+h_create()
+{
+ local dev=$1; shift
+ local ip=$1; shift
+ local ip6=$1
+
+ simple_if_init $dev $ip $ip6
+}
+
+h_destroy()
+{
+ local dev=$1; shift
+ local ip=$1; shift
+ local ip6=$1
+
+ simple_if_fini $dev $ip $ip6
+}
+
+switch_create()
+{
+ ip link add dev br0 type bridge vlan_filtering 1 vlan_protocol 802.1q \
+ mcast_snooping 0
+
+ ip link set dev $swp1 master br0
+ ip link set dev $swp2 master br0
+
+ ip link set dev br0 up
+ ip link set dev $swp1 up
+ ip link set dev $swp2 up
+
+ bridge vlan add dev $swp1 vid $svid master
+ bridge vlan add dev br0 vid $svid self
+ bridge vlan add dev $swp2 vid $svid master
+}
+
+switch_destroy()
+{
+ ip link set dev $swp2 down
+ ip link set dev $swp1 down
+
+ ip link del dev br0
+}
+
+tc_vlan_create()
+{
+ tc qdisc add dev $swp1 clsact
+
+ tc filter add dev $swp1 ingress pref 1 protocol all flower skip_hw \
+ action vlan push id $cvid protocol 802.1q pipe \
+ action vlan push id $svid protocol 802.1q
+
+ tc filter add dev $swp1 egress pref 1 protocol 802.1q \
+ flower skip_hw vlan_id $svid \
+ vlan_ethtype 802.1q cvlan_id $cvid \
+ action vlan pop pipe action vlan pop
+}
+
+tc_vlan_destroy()
+{
+ tc filter del dev $swp1 ingress pref 1
+ tc filter del dev $swp1 egress pref 1
+ tc qdisc del dev $swp1 clsact
+}
+
+vlan_create()
+{
+ local dev=$1; shift
+ local vid=$1; shift
+ local tpid=$1;
+
+ ip link add link $dev name $dev.$vid type vlan id $vid proto $tpid
+ ip link set dev $dev up
+ ip link set dev $dev.$vid
+}
+
+vlan_destroy()
+{
+ local dev=$1
+
+ ip link del dev $dev
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ swp2=${NETIFS[p3]}
+ h2=${NETIFS[p4]}
+
+ cvid=10
+ svid=100
+
+ vrf_prepare
+
+ switch_create
+
+ tc_vlan_create
+
+ h_create $h1 192.0.2.1/24 2001:db8:1::1/64
+
+ vlan_create $h2 $svid 802.1q
+ vlan_create $h2.$svid $cvid 802.1q
+
+ h_create $h2.$svid.$cvid 192.0.2.2/24 2001:db8:1::2/64
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ tc_vlan_destroy
+
+ switch_destroy
+
+ h_destroy $h1 192.0.2.1/24 2001:db8:1::1/64
+ h_destroy $h2.$svid.$cvid 192.0.2.2/24 2001:db8:1::2/64
+
+ vlan_destroy $h2.$svid.$cvid
+ vlan_destroy $h2.$svid
+
+ ip link del dev $h1
+ ip link del dev $h2
+
+ vrf_cleanup
+}
+
+test_tc_vlan_bridge_ipv4_forwarding()
+{
+ ip link set dev br0 type bridge vlan_protocol 802.1q
+ ping_do $h1 192.0.2.2
+ check_err $? "Packets were not forwarded"
+ log_test "IPv4 tc-vlan bridge forwarding"
+}
+
+test_tc_vlan_bridge_ipv4_forwarding_proto()
+{
+ ip link set dev br0 type bridge vlan_protocol 802.1ad
+ ping_do $h1 192.0.2.2
+ check_err $? "Packets were not forwarded"
+ log_test "IPv4 tc-vlan bridge forwarding protocol mismatch"
+}
+
+test_tc_vlan_bridge_ipv6_forwarding()
+{
+ ip link set dev br0 type bridge vlan_protocol 802.1q
+ ping6_do $h1 2001:db8:1::2
+ check_err $? "Packets were not forwarded"
+ log_test "IPv6 tc-vlan bridge forwarding"
+}
+
+test_tc_vlan_bridge_ipv6_forwarding_proto()
+{
+ ip link set dev br0 type bridge vlan_protocol 802.1ad
+ ping6_do $h1 2001:db8:1::2
+ check_err $? "Packet were not forwarded"
+ log_test "IPv6 tc-vlan bridge forwarding protocol mismatch"
+}
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.22.0
^ permalink raw reply related
* [PATCH][next] net/sched: cbs: remove redundant assignment to variable port_rate
From: Colin King @ 2019-09-02 18:26 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Variable port_rate is being initialized with a value that is never read
and is being re-assigned a little later on. The assignment is redundant
and hence can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/sched/sch_cbs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index 810645b5c086..93b58fde99b7 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -299,7 +299,7 @@ static void cbs_set_port_rate(struct net_device *dev, struct cbs_sched_data *q)
{
struct ethtool_link_ksettings ecmd;
int speed = SPEED_10;
- int port_rate = -1;
+ int port_rate;
int err;
err = __ethtool_get_link_ksettings(dev, &ecmd);
--
2.20.1
^ permalink raw reply related
* Re: [net-next 1/3] ravb: correct typo in FBP field of SFO register
From: David Miller @ 2019-09-02 18:33 UTC (permalink / raw)
To: horms+renesas
Cc: sergei.shtylyov, magnus.damm, netdev, linux-renesas-soc,
kazuya.mizuguchi.ks
In-Reply-To: <20190902080603.5636-2-horms+renesas@verge.net.au>
From: Simon Horman <horms+renesas@verge.net.au>
Date: Mon, 2 Sep 2019 10:06:01 +0200
> - SFO_FPB = 0x0000003F,
> + SFO_FBP = 0x0000003F,
> };
>
> /* RTC */
> ---
> drivers/net/ethernet/renesas/ravb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
Simon please clean this up, I don't know what happened here :-)
^ permalink raw reply
* Re: [PATCH net-next] r8152: modify rtl8152_set_speed function
From: Heiner Kallweit @ 2019-09-02 18:36 UTC (permalink / raw)
To: Hayes Wang, netdev; +Cc: nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-326-Taiwan-albertk@realtek.com>
On 02.09.2019 13:52, Hayes Wang wrote:
> First, for AUTONEG_DISABLE, we only need to modify MII_BMCR.
>
> Second, add advertising parameter for rtl8152_set_speed(). Add
> RTL_ADVERTISED_xxx for advertising parameter of rtl8152_set_speed().
> Then, the advertising settings from ethtool could be saved.
>
Seeing all this code it might be a good idea to switch this driver
to phylib, similar to what I did with r8169 some time ago.
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> drivers/net/usb/r8152.c | 196 +++++++++++++++++++++++++++-------------
> 1 file changed, 132 insertions(+), 64 deletions(-)
>
[...]
^ permalink raw reply
* Re: [PATCH net-next] net_failover: get rid of the limitaion receive packet from standy dev when primary exist
From: David Miller @ 2019-09-02 18:42 UTC (permalink / raw)
To: wenxu; +Cc: sridhar.samudrala, netdev
In-Reply-To: <1567141668-12196-1-git-send-email-wenxu@ucloud.cn>
From: wenxu@ucloud.cn
Date: Fri, 30 Aug 2019 13:07:48 +0800
> From: wenxu <wenxu@ucloud.cn>
>
> For receive side the standby, primary and failover is the same one,
> If the packet receive from standby or primary should can be deliver
> to failover dev.
>
> For example: there are VF and virtio device failover together.
> When live migration the VF detached and send/recv packet through
> virtio device. When VF attached again some ingress traffic may
> receive from virtio device for cache reason(TC flower offload in
> sw mode).
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
I don't understand, the device logic guarding the device rewriting in
this handler looks very much intentional.
After your changes we have no logic at all, we just unconditionally
always rewrite.
^ permalink raw reply
* Re: [patch net-next v2] mlx5: Add missing init_net check in FIB notifier
From: David Miller @ 2019-09-02 18:45 UTC (permalink / raw)
To: jiri; +Cc: netdev, saeedm, leon, roid, mlxsw
In-Reply-To: <20190830082530.8958-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 30 Aug 2019 10:25:30 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Take only FIB events that are happening in init_net into account. No other
> namespaces are supported.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v1->v2:
> - no change, just cced maintainers (fat finger made me avoid them in v1)
Applied, thanks Jiri.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Jiri Pirko @ 2019-09-02 18:45 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: David Miller, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190902180519.ytbs6x2dx5z23hys@lx-anielsen.microsemi.net>
Mon, Sep 02, 2019 at 08:05:20PM CEST, allan.nielsen@microchip.com wrote:
>The 09/02/2019 19:51, Jiri Pirko wrote:
>> External E-Mail
>>
>>
>> Mon, Sep 02, 2019 at 07:42:31PM CEST, allan.nielsen@microchip.com wrote:
>> >Hi Jiri,
>> >
>> >Sorry for joining the discussion this late, but I have been without mail access
>> >for the last few days.
>> >
>> >
>> >The 08/30/2019 08:36, Jiri Pirko wrote:
>> >> Fri, Aug 30, 2019 at 08:02:33AM CEST, davem@davemloft.net wrote:
>> >> >From: Jiri Pirko <jiri@resnulli.us>
>> >> >Date: Fri, 30 Aug 2019 07:39:40 +0200
>> >> >
>> >> >> Because the "promisc mode" would gain another meaning. Now how the
>> >> >> driver should guess which meaning the user ment when he setted it?
>> >> >> filter or trap?
>> >> >>
>> >> >> That is very confusing. If the flag is the way to do this, let's
>> >> >> introduce another flag, like IFF_TRAPPING indicating that user wants
>> >> >> exactly this.
>> >> >
>> >> >I don't understand how the meaning of promiscuous mode for a
>> >> >networking device has suddenly become ambiguous, when did this start
>> >> >happening?
>> >>
>> >> The promiscuity is a way to setup the rx filter. So promics == rx filter
>> >> off. For normal nics, where there is no hw fwd datapath,
>> >> this coincidentally means all received packets go to cpu.
>> >> But if there is hw fwd datapath, rx filter is still off, all rxed packets
>> >> are processed. But that does not mean they should be trapped to cpu.
>> >>
>> >> Simple example:
>> >> I need to see slowpath packets, for example arps/stp/bgp/... that
>> >> are going to cpu, I do:
>> >> tcpdump -i swp1
>> >
>> >How is this different from "tcpdump -p -i swp1"
>> >
>> >> I don't want to get all the traffic running over hw running this cmd.
>> >> This is a valid usecase.
>> >>
>> >> To cope with hw fwd datapath devices, I believe that tcpdump has to have
>> >> notion of that. Something like:
>> >>
>> >> tcpdump -i swp1 --hw-trapping-mode
>> >>
>> >> The logic can be inverse:
>> >> tcpdump -i swp1
>> >> tcpdump -i swp1 --no-hw-trapping-mode
>> >>
>> >> However, that would provide inconsistent behaviour between existing and
>> >> patched tcpdump/kernel.
>> >>
>> >> All I'm trying to say, there are 2 flags
>> >> needed (if we don't use tc trap).
>> >
>> >I have been reading through this thread several times and I still do not get it.
>> >
>> >As far as I understand you are arguing that we need 3 modes:
>> >
>> >- tcpdump -i swp1
>>
>> Depends on default. Promisc is on.
>>
>>
>> >- tcpdump -p -i swp1
>>
>> All traffic that is trapped to the cpu by default, not promisc means
>> only mac of the interface (if bridge for example haven't set promisc
>> already) and special macs. So host traffic (ip of host), bgp, arp, nsnd,
>> etc.
>
>In the case where the interface is enslaved to a bridge, it is put into promisc
>mode, which means that "tcpdump -i swp1" and "tcpdump -p -i swp1" give the same
>result, right?
>
>Is this desirable?
Yes, that is correct and expected. It it might not be bridged, depends
on a usecase.
>
>> >- tcpdump -i swp1 --hw-trapping-mode
>>
>> Promisc is on, all traffic received on the port and pushed to cpu. User
>> has to be careful because in case of mlxsw this can lead to couple
>> hundred gigabit traffic going over limited pci bandwidth (gigabits).
>>
>>
>> >
>> >Would you mind provide an example of the traffic you want to see in the 3 cases
>> >(or the traffic which you do not want to see).
>> >
>> >/Allan
>> >
>>
>
>--
>/Allan
^ permalink raw reply
* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
From: Greg KH @ 2019-09-02 18:47 UTC (permalink / raw)
To: Guenter Roeck
Cc: Kalle Valo, Hui Peng, security, Mathias Payer, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <385361d3-048e-9b3f-c749-aa5861e397e7@roeck-us.net>
On Sun, Sep 01, 2019 at 07:08:29AM -0700, Guenter Roeck wrote:
> On 9/1/19 1:03 AM, Kalle Valo wrote:
> > Guenter Roeck <linux@roeck-us.net> writes:
> >
> > > On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
> > > > `dev` (struct rsi_91x_usbdev *) field of adapter
> > > > (struct rsi_91x_usbdev *) is allocated and initialized in
> > > > `rsi_init_usb_interface`. If any error is detected in information
> > > > read from the device side, `rsi_init_usb_interface` will be
> > > > freed. However, in the higher level error handling code in
> > > > `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
> > > > again, in which `dev` will be freed again, resulting double free.
> > > >
> > > > This patch fixes the double free by removing the free operation on
> > > > `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
> > > > used in `rsi_disconnect`, in that code path, the `dev` field is not
> > > > (and thus needs to be) freed.
> > > >
> > > > This bug was found in v4.19, but is also present in the latest version
> > > > of kernel.
> > > >
> > > > Reported-by: Hui Peng <benquike@gmail.com>
> > > > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > > > Signed-off-by: Hui Peng <benquike@gmail.com>
> > >
> > > FWIW:
> > >
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > >
> > > This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> > > of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
> >
> > A double free in error path is considered as a critical CVE issue? I'm
> > very curious, why is that?
> >
>
> You'd have to ask the people assigning CVSS scores. However, if the memory
> was reallocated, that reallocated memory (which is still in use) is freed.
> Then all kinds of bad things can happen.
Yes, but moving from "bad things _can_ happen" to "bad things happen" in
an instance like this will be a tough task. It also requires physical
access to the machine.
Anyway, that doesn't mean we shouldn't fix it, it's just that CVSS can
be crazy when it comes to kernel patches (i.e. almost all fixes should
be "critical"...)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] net: stmmac: dwmac-sun8i: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
From: David Miller @ 2019-09-02 18:48 UTC (permalink / raw)
To: yzhai003
Cc: csong, zhiyunq, peppe.cavallaro, alexandre.torgue, joabreu,
mripard, wens, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-stm32, linux-kernel
In-Reply-To: <20190831020049.6516-1-yzhai003@ucr.edu>
From: Yizhuo <yzhai003@ucr.edu>
Date: Fri, 30 Aug 2019 19:00:48 -0700
> In function sun8i_dwmac_set_syscon(), local variable "val" could
> be uninitialized if function regmap_field_read() returns -EINVAL.
> However, it will be used directly in the if statement, which
> is potentially unsafe.
>
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] net: dsa: Fix off-by-one number of calls to devlink_port_unregister
From: David Miller @ 2019-09-02 19:00 UTC (permalink / raw)
To: olteanv; +Cc: f.fainelli, vivien.didelot, andrew, netdev
In-Reply-To: <20190831124619.460-1-olteanv@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com>
Date: Sat, 31 Aug 2019 15:46:19 +0300
> When a function such as dsa_slave_create fails, currently the following
> stack trace can be seen:
...
> devlink_free is complaining right here:
>
> WARN_ON(!list_empty(&devlink->port_list));
>
> This happens because devlink_port_unregister is no longer done right
> away in dsa_port_setup when a DSA_PORT_TYPE_USER has failed.
> Vivien said about this change that:
>
> Also no need to call devlink_port_unregister from within dsa_port_setup
> as this step is inconditionally handled by dsa_port_teardown on error.
>
> which is not really true. The devlink_port_unregister function _is_
> being called unconditionally from within dsa_port_setup, but not for
> this port that just failed, just for the previous ones which were set
> up.
>
> ports_teardown:
> for (i = 0; i < port; i++)
> dsa_port_teardown(&ds->ports[i]);
>
> Initially I was tempted to fix this by extending the "for" loop to also
> cover the port that failed during setup. But this could have potentially
> unforeseen consequences unrelated to devlink_port or even other types of
> ports than user ports, which I can't really test for. For example, if
> for some reason devlink_port_register itself would fail, then
> unconditionally unregistering it in dsa_port_teardown would not be a
> smart idea. The list might go on.
>
> So just make dsa_port_setup undo the setup it had done upon failure, and
> let the for loop undo the work of setting up the previous ports, which
> are guaranteed to be brought up to a consistent state.
>
> Fixes: 955222ca5281 ("net: dsa: use a single switch statement for port setup")
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Applied to net-next, thanks.
^ 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