* Re: [PATCH net-next,v2 09/12] flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure translator
From: Jiri Pirko @ 2018-11-19 14:49 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz
In-Reply-To: <20181119001519.12124-10-pablo@netfilter.org>
Mon, Nov 19, 2018 at 01:15:16AM CET, pablo@netfilter.org wrote:
>This patch adds a function to translate the ethtool_rx_flow_spec
>structure to the flow_rule representation.
>
>This allows us to reuse code from the driver side given that both flower
>and ethtool_rx_flow interfaces use the same representation.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
>v2: no changes.
>
> include/net/flow_dissector.h | 5 ++
> net/core/flow_dissector.c | 190 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 195 insertions(+)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 7a4683646d5a..ec9036232538 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -485,4 +485,9 @@ static inline bool flow_rule_match_key(const struct flow_rule *rule,
> return dissector_uses_key(rule->match.dissector, key);
> }
>
>+struct ethtool_rx_flow_spec;
>+
>+struct flow_rule *ethtool_rx_flow_rule(const struct ethtool_rx_flow_spec *fs);
>+void ethtool_rx_flow_rule_free(struct flow_rule *rule);
>+
> #endif
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index b9368349f0f7..ef5bdb62620c 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -17,6 +17,7 @@
> #include <linux/dccp.h>
> #include <linux/if_tunnel.h>
> #include <linux/if_pppox.h>
>+#include <uapi/linux/ethtool.h>
> #include <linux/ppp_defs.h>
> #include <linux/stddef.h>
> #include <linux/if_ether.h>
>@@ -276,6 +277,195 @@ void flow_action_free(struct flow_action *flow_action)
> }
> EXPORT_SYMBOL(flow_action_free);
>
>+struct ethtool_rx_flow_key {
>+ struct flow_dissector_key_basic basic;
>+ union {
>+ struct flow_dissector_key_ipv4_addrs ipv4;
>+ struct flow_dissector_key_ipv6_addrs ipv6;
>+ };
>+ struct flow_dissector_key_ports tp;
>+ struct flow_dissector_key_ip ip;
>+} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>+
>+struct ethtool_rx_flow_match {
>+ struct flow_dissector dissector;
>+ struct ethtool_rx_flow_key key;
>+ struct ethtool_rx_flow_key mask;
>+};
>+
>+struct flow_rule *ethtool_rx_flow_rule(const struct ethtool_rx_flow_spec *fs)
>+{
>+ static struct in6_addr zero_addr = {};
>+ struct ethtool_rx_flow_match *match;
>+ struct flow_action_key *act;
>+ struct flow_rule *rule;
>+
>+ rule = kmalloc(sizeof(struct flow_rule), GFP_KERNEL);
Allocating struct flow_rule manually here seems wrong. There should
be some helpers. Please see below.***
>+ if (!rule)
>+ return NULL;
>+
>+ match = kzalloc(sizeof(struct ethtool_rx_flow_match), GFP_KERNEL);
>+ if (!match)
>+ goto err_match;
>+
>+ rule->match.dissector = &match->dissector;
>+ rule->match.mask = &match->mask;
>+ rule->match.key = &match->key;
>+
>+ match->mask.basic.n_proto = 0xffff;
>+
>+ switch (fs->flow_type & ~FLOW_EXT) {
>+ case TCP_V4_FLOW:
>+ case UDP_V4_FLOW: {
>+ const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
>+
>+ match->key.basic.n_proto = htons(ETH_P_IP);
>+
>+ v4_spec = &fs->h_u.tcp_ip4_spec;
>+ v4_m_spec = &fs->m_u.tcp_ip4_spec;
>+
>+ if (v4_m_spec->ip4src) {
>+ match->key.ipv4.src = v4_spec->ip4src;
>+ match->mask.ipv4.src = v4_m_spec->ip4src;
>+ }
>+ if (v4_m_spec->ip4dst) {
>+ match->key.ipv4.dst = v4_spec->ip4dst;
>+ match->mask.ipv4.dst = v4_m_spec->ip4dst;
>+ }
>+ if (v4_m_spec->ip4src ||
>+ v4_m_spec->ip4dst) {
>+ match->dissector.used_keys |=
>+ FLOW_DISSECTOR_KEY_IPV4_ADDRS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV4_ADDRS] =
>+ offsetof(struct ethtool_rx_flow_key, ipv4);
>+ }
>+ if (v4_m_spec->psrc) {
>+ match->key.tp.src = v4_spec->psrc;
>+ match->mask.tp.src = v4_m_spec->psrc;
>+ }
>+ if (v4_m_spec->pdst) {
>+ match->key.tp.dst = v4_spec->pdst;
>+ match->mask.tp.dst = v4_m_spec->pdst;
>+ }
>+ if (v4_m_spec->psrc ||
>+ v4_m_spec->pdst) {
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_PORTS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
>+ offsetof(struct ethtool_rx_flow_key, tp);
>+ }
>+ if (v4_m_spec->tos) {
>+ match->key.ip.tos = v4_spec->pdst;
>+ match->mask.ip.tos = v4_m_spec->pdst;
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_IP;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
>+ offsetof(struct ethtool_rx_flow_key, ip);
>+ }
>+ }
>+ break;
>+ case TCP_V6_FLOW:
>+ case UDP_V6_FLOW: {
>+ const struct ethtool_tcpip6_spec *v6_spec, *v6_m_spec;
>+
>+ match->key.basic.n_proto = htons(ETH_P_IPV6);
>+
>+ v6_spec = &fs->h_u.tcp_ip6_spec;
>+ v6_m_spec = &fs->m_u.tcp_ip6_spec;
>+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
>+ memcpy(&match->key.ipv6.src, v6_spec->ip6src,
>+ sizeof(match->key.ipv6.src));
>+ memcpy(&match->mask.ipv6.src, v6_m_spec->ip6src,
>+ sizeof(match->mask.ipv6.src));
>+ }
>+ if (memcmp(v6_m_spec->ip6dst, &zero_addr, sizeof(zero_addr))) {
>+ memcpy(&match->key.ipv6.dst, v6_spec->ip6dst,
>+ sizeof(match->key.ipv6.dst));
>+ memcpy(&match->mask.ipv6.dst, v6_m_spec->ip6dst,
>+ sizeof(match->mask.ipv6.dst));
>+ }
>+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr)) ||
>+ memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
>+ match->dissector.used_keys |=
>+ FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV6_ADDRS] =
>+ offsetof(struct ethtool_rx_flow_key, ipv6);
>+ }
>+ if (v6_m_spec->psrc) {
>+ match->key.tp.src = v6_spec->psrc;
>+ match->mask.tp.src = v6_m_spec->psrc;
>+ }
>+ if (v6_m_spec->pdst) {
>+ match->key.tp.dst = v6_spec->pdst;
>+ match->mask.tp.dst = v6_m_spec->pdst;
>+ }
>+ if (v6_m_spec->psrc ||
>+ v6_m_spec->pdst) {
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_PORTS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
>+ offsetof(struct ethtool_rx_flow_key, tp);
>+ }
>+ if (v6_m_spec->tclass) {
>+ match->key.ip.tos = v6_spec->tclass;
>+ match->mask.ip.tos = v6_m_spec->tclass;
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_IP;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
>+ offsetof(struct ethtool_rx_flow_key, ip);
>+ }
>+ }
>+ break;
>+ }
>+
>+ switch (fs->flow_type & ~FLOW_EXT) {
>+ case TCP_V4_FLOW:
>+ case TCP_V6_FLOW:
>+ match->key.basic.ip_proto = IPPROTO_TCP;
>+ break;
>+ case UDP_V4_FLOW:
>+ case UDP_V6_FLOW:
>+ match->key.basic.ip_proto = IPPROTO_UDP;
>+ break;
>+ }
>+ match->mask.basic.ip_proto = 0xff;
>+
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_BASIC;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_BASIC] =
>+ offsetof(struct ethtool_rx_flow_key, basic);
>+
>+ /* ethtool_rx supports only one single action per rule. */
>+ if (flow_action_init(&rule->action, 1) < 0)
>+ goto err_action;
>+
>+ act = &rule->action.keys[0];
>+ switch (fs->ring_cookie) {
>+ case RX_CLS_FLOW_DISC:
>+ act->id = FLOW_ACTION_KEY_DROP;
>+ break;
>+ case RX_CLS_FLOW_WAKE:
>+ act->id = FLOW_ACTION_KEY_WAKE;
>+ break;
>+ default:
>+ act->id = FLOW_ACTION_KEY_QUEUE;
>+ act->queue_index = fs->ring_cookie;
>+ break;
>+ }
>+
>+ return rule;
>+
>+err_action:
>+ kfree(match);
>+err_match:
>+ kfree(rule);
>+ return NULL;
>+}
>+EXPORT_SYMBOL(ethtool_rx_flow_rule);
>+
>+void ethtool_rx_flow_rule_free(struct flow_rule *rule)
>+{
>+ kfree((struct flow_match *)rule->match.dissector);
Ouch. I wonder if it cannot be stored rather in some rule->priv or
something.
On alloc, you can have a helper to allocate both:
***
struct flow_rule {
...
unsigned long priv[0];
};
struct flow_rule *flow_rule_alloc(size_t priv_size)
{
return kzalloc(sizeof(struct flow_rule) + priv_size, ...);
}
>+ flow_action_free(&rule->action);
>+ kfree(rule);
>+}
>+EXPORT_SYMBOL(ethtool_rx_flow_rule_free);
>+
> /**
> * __skb_flow_get_ports - extract the upper layer ports and return them
> * @skb: sk_buff to extract the ports from
>--
>2.11.0
>
^ permalink raw reply
* [PATCH v3 1/5] phy: core: rework phy_set_mode to accept phy mode and submode
From: Grygorii Strashko @ 2018-11-20 1:24 UTC (permalink / raw)
To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko
In-Reply-To: <20181120012424.11802-1-grygorii.strashko@ti.com>
Currently the attempt to add support for Ethernet interface mode PHY
(MII/GMII/RGMII) will lead to the necessity of extending enum phy_mode and
duplicate there values from phy_interface_t enum (or introduce more PHY
callbacks) [1]. Both approaches are ineffective and would lead to fast
bloating of enum phy_mode or struct phy_ops in the process of adding more
PHYs for different subsystems which will make them unmaintainable.
As discussed in [1] the solution could be to introduce dual level PHYs mode
configuration - PHY mode and PHY submode. The PHY mode will define generic
PHY type (subsystem - PCIE/ETHERNET/USB_) while the PHY submode - subsystem
specific interface mode. The last is usually already defined in
corresponding subsystem headers (phy_interface_t for Ethernet, enum
usb_device_speed for USB).
This patch is cumulative change which refactors PHY framework code to
support dual level PHYs mode configuration - PHY mode and PHY submode. It
extends .set_mode() callback to support additional parameter "int submode"
and converts all corresponding PHY drivers to support new .set_mode()
callback declaration.
The new extended PHY API
int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
is introduced to support dual level PHYs mode configuration and existing
phy_set_mode() API is converted to macros, so PHY framework consumers do
not need to be changed (~21 matches).
[1] https://lkml.org/lkml/2018/10/25/366
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
drivers/phy/allwinner/phy-sun4i-usb.c | 3 ++-
drivers/phy/amlogic/phy-meson-gxl-usb2.c | 5 +++--
drivers/phy/amlogic/phy-meson-gxl-usb3.c | 5 +++--
drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 3 ++-
drivers/phy/mediatek/phy-mtk-tphy.c | 2 +-
drivers/phy/mediatek/phy-mtk-xsphy.c | 2 +-
drivers/phy/mscc/phy-ocelot-serdes.c | 2 +-
drivers/phy/phy-core.c | 6 +++---
drivers/phy/qualcomm/phy-qcom-qmp.c | 3 ++-
drivers/phy/qualcomm/phy-qcom-qusb2.c | 3 ++-
drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 3 ++-
drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 3 ++-
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 3 ++-
drivers/phy/ti/phy-da8xx-usb.c | 3 ++-
drivers/phy/ti/phy-tusb1210.c | 2 +-
include/linux/phy/phy.h | 13 ++++++++++---
16 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index d4dcd39..1acdd73 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -474,7 +474,8 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
return 0;
}
-static int sun4i_usb_phy_set_mode(struct phy *_phy, enum phy_mode mode)
+static int sun4i_usb_phy_set_mode(struct phy *_phy,
+ enum phy_mode mode, int submode)
{
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb2.c b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
index 9f9b541..148ef0b 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
@@ -152,7 +152,8 @@ static int phy_meson_gxl_usb2_reset(struct phy *phy)
return 0;
}
-static int phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode)
+static int phy_meson_gxl_usb2_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
@@ -209,7 +210,7 @@ static int phy_meson_gxl_usb2_power_on(struct phy *phy)
/* power on the PHY by taking it out of reset mode */
regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET, 0);
- ret = phy_meson_gxl_usb2_set_mode(phy, priv->mode);
+ ret = phy_meson_gxl_usb2_set_mode(phy, priv->mode, 0);
if (ret) {
phy_meson_gxl_usb2_power_off(phy);
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb3.c b/drivers/phy/amlogic/phy-meson-gxl-usb3.c
index d37d94d..c0e9e4c 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb3.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb3.c
@@ -119,7 +119,8 @@ static int phy_meson_gxl_usb3_power_off(struct phy *phy)
return 0;
}
-static int phy_meson_gxl_usb3_set_mode(struct phy *phy, enum phy_mode mode)
+static int phy_meson_gxl_usb3_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
@@ -164,7 +165,7 @@ static int phy_meson_gxl_usb3_init(struct phy *phy)
if (ret)
goto err_disable_clk_phy;
- ret = phy_meson_gxl_usb3_set_mode(phy, priv->mode);
+ ret = phy_meson_gxl_usb3_set_mode(phy, priv->mode, 0);
if (ret)
goto err_disable_clk_peripheral;
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 86a5f7b..79b52c3 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -512,7 +512,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
return ret;
}
-static int mvebu_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mvebu_comphy_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index 3eb8e1b..5b6a470 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -971,7 +971,7 @@ static int mtk_phy_exit(struct phy *phy)
return 0;
}
-static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
{
struct mtk_phy_instance *instance = phy_get_drvdata(phy);
struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
diff --git a/drivers/phy/mediatek/phy-mtk-xsphy.c b/drivers/phy/mediatek/phy-mtk-xsphy.c
index 020cd02..8c51131 100644
--- a/drivers/phy/mediatek/phy-mtk-xsphy.c
+++ b/drivers/phy/mediatek/phy-mtk-xsphy.c
@@ -426,7 +426,7 @@ static int mtk_phy_exit(struct phy *phy)
return 0;
}
-static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
{
struct xsphy_instance *inst = phy_get_drvdata(phy);
struct mtk_xsphy *xsphy = dev_get_drvdata(phy->dev.parent);
diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index cbb49d9..c61a9890 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -158,7 +158,7 @@ static const struct serdes_mux ocelot_serdes_muxes[] = {
HSIO_HW_CFG_PCIE_ENA),
};
-static int serdes_set_mode(struct phy *phy, enum phy_mode mode)
+static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
{
struct serdes_macro *macro = phy_get_drvdata(phy);
unsigned int i;
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 35fd38c..df3d4ba 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -360,7 +360,7 @@ int phy_power_off(struct phy *phy)
}
EXPORT_SYMBOL_GPL(phy_power_off);
-int phy_set_mode(struct phy *phy, enum phy_mode mode)
+int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
{
int ret;
@@ -368,14 +368,14 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
return 0;
mutex_lock(&phy->mutex);
- ret = phy->ops->set_mode(phy, mode);
+ ret = phy->ops->set_mode(phy, mode, submode);
if (!ret)
phy->attrs.mode = mode;
mutex_unlock(&phy->mutex);
return ret;
}
-EXPORT_SYMBOL_GPL(phy_set_mode);
+EXPORT_SYMBOL_GPL(phy_set_mode_ext);
int phy_reset(struct phy *phy)
{
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index a833324..514db72 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -1365,7 +1365,8 @@ static int qcom_qmp_phy_poweron(struct phy *phy)
return ret;
}
-static int qcom_qmp_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qcom_qmp_phy_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct qmp_phy *qphy = phy_get_drvdata(phy);
struct qcom_qmp *qmp = qphy->qmp;
diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 9ce5311..098d793 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -423,7 +423,8 @@ static void qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
}
-static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qusb2_phy_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct qusb2_phy *qphy = phy_get_drvdata(phy);
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
index ba1895b..1e0d4f2 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
@@ -65,7 +65,8 @@ static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)
}
static
-int ufs_qcom_phy_qmp_14nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+int ufs_qcom_phy_qmp_14nm_set_mode(struct phy *generic_phy,
+ enum phy_mode mode, int submode)
{
struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
index 49f435c..aef40f7 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
@@ -84,7 +84,8 @@ static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)
}
static
-int ufs_qcom_phy_qmp_20nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+int ufs_qcom_phy_qmp_20nm_set_mode(struct phy *generic_phy,
+ enum phy_mode mode, int submode)
{
struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index abbbe75..04934f8 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -42,7 +42,8 @@ struct qcom_usb_hs_phy {
struct notifier_block vbus_notify;
};
-static int qcom_usb_hs_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qcom_usb_hs_phy_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
u8 addr;
diff --git a/drivers/phy/ti/phy-da8xx-usb.c b/drivers/phy/ti/phy-da8xx-usb.c
index befb886..d5f4fbc 100644
--- a/drivers/phy/ti/phy-da8xx-usb.c
+++ b/drivers/phy/ti/phy-da8xx-usb.c
@@ -93,7 +93,8 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)
return 0;
}
-static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int da8xx_usb20_phy_set_mode(struct phy *phy,
+ enum phy_mode mode, int submode)
{
struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);
u32 val;
diff --git a/drivers/phy/ti/phy-tusb1210.c b/drivers/phy/ti/phy-tusb1210.c
index b8ec39a..329fb93 100644
--- a/drivers/phy/ti/phy-tusb1210.c
+++ b/drivers/phy/ti/phy-tusb1210.c
@@ -53,7 +53,7 @@ static int tusb1210_power_off(struct phy *phy)
return 0;
}
-static int tusb1210_set_mode(struct phy *phy, enum phy_mode mode)
+static int tusb1210_set_mode(struct phy *phy, enum phy_mode mode, int submode)
{
struct tusb1210 *tusb = phy_get_drvdata(phy);
int ret;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03b319f..b17e770 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -60,7 +60,7 @@ struct phy_ops {
int (*exit)(struct phy *phy);
int (*power_on)(struct phy *phy);
int (*power_off)(struct phy *phy);
- int (*set_mode)(struct phy *phy, enum phy_mode mode);
+ int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
int (*reset)(struct phy *phy);
int (*calibrate)(struct phy *phy);
struct module *owner;
@@ -164,7 +164,10 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
-int phy_set_mode(struct phy *phy, enum phy_mode mode);
+int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
+#define phy_set_mode(phy, mode) \
+ phy_set_mode_ext(phy, mode, 0)
+
static inline enum phy_mode phy_get_mode(struct phy *phy)
{
return phy->attrs.mode;
@@ -278,13 +281,17 @@ static inline int phy_power_off(struct phy *phy)
return -ENOSYS;
}
-static inline int phy_set_mode(struct phy *phy, enum phy_mode mode)
+static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
+ int submode)
{
if (!phy)
return 0;
return -ENOSYS;
}
+#define phy_set_mode(phy, mode) \
+ phy_set_mode_ext(phy, mode, 0)
+
static inline enum phy_mode phy_get_mode(struct phy *phy)
{
return PHY_MODE_INVALID;
--
2.10.5
^ permalink raw reply related
* [PATCH v3 2/5] phy: core: add PHY_MODE_ETHERNET
From: Grygorii Strashko @ 2018-11-20 1:24 UTC (permalink / raw)
To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko
In-Reply-To: <20181120012424.11802-1-grygorii.strashko@ti.com>
Add new PHY's mode to be used by Ethernet PHY interface drivers or
multipurpose PHYs like serdes. It will be reused in further changes.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
include/linux/phy/phy.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b17e770..02c9ef0 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,6 +42,7 @@ enum phy_mode {
PHY_MODE_UFS_HS_A,
PHY_MODE_UFS_HS_B,
PHY_MODE_PCIE,
+ PHY_MODE_ETHERNET,
};
/**
--
2.10.5
^ permalink raw reply related
* [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode
From: Grygorii Strashko @ 2018-11-20 1:24 UTC (permalink / raw)
To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: Alexandre Belloni, Grygorii Strashko, Manu Gautam, Tony Lindgren,
netdev-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
Chunfeng Yun, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Vivek Gautam, Carlo Caione,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Matthias Brugger
In-Reply-To: <20181120012424.11802-1-grygorii.strashko-l0cyMroinI0@public.gmane.org>
Convert ocelot-serdes PHY driver to use recently introduced
PHY_MODE_ETHERNET and phy_set_mode_ext().
Cc: Quentin Schulz <quentin.schulz-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
drivers/net/ethernet/mscc/ocelot.c | 9 ++-------
drivers/phy/mscc/phy-ocelot-serdes.c | 22 ++++++++++++++++------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 3238b9e..3edb608 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -472,7 +472,6 @@ static int ocelot_port_open(struct net_device *dev)
{
struct ocelot_port *port = netdev_priv(dev);
struct ocelot *ocelot = port->ocelot;
- enum phy_mode phy_mode;
int err;
/* Enable receiving frames on the port, and activate auto-learning of
@@ -484,12 +483,8 @@ static int ocelot_port_open(struct net_device *dev)
ANA_PORT_PORT_CFG, port->chip_port);
if (port->serdes) {
- if (port->phy_mode == PHY_INTERFACE_MODE_SGMII)
- phy_mode = PHY_MODE_SGMII;
- else
- phy_mode = PHY_MODE_QSGMII;
-
- err = phy_set_mode(port->serdes, phy_mode);
+ err = phy_set_mode_ext(port->serdes, PHY_MODE_ETHERNET,
+ port->phy_mode);
if (err) {
netdev_err(dev, "Could not set mode of SerDes\n");
return err;
diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index c61a9890..77c46f6 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/phy.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -104,20 +105,24 @@ struct serdes_mux {
u8 idx;
u8 port;
enum phy_mode mode;
+ int submode;
u32 mask;
u32 mux;
};
-#define SERDES_MUX(_idx, _port, _mode, _mask, _mux) { \
+#define SERDES_MUX(_idx, _port, _mode, _submode, _mask, _mux) { \
.idx = _idx, \
.port = _port, \
.mode = _mode, \
+ .submode = _submode, \
.mask = _mask, \
.mux = _mux, \
}
-#define SERDES_MUX_SGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_SGMII, m, c)
-#define SERDES_MUX_QSGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_QSGMII, m, c)
+#define SERDES_MUX_SGMII(i, p, m, c) \
+ SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_SGMII, m, c)
+#define SERDES_MUX_QSGMII(i, p, m, c) \
+ SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_QSGMII, m, c)
static const struct serdes_mux ocelot_serdes_muxes[] = {
SERDES_MUX_SGMII(SERDES1G(0), 0, 0, 0),
@@ -154,7 +159,7 @@ static const struct serdes_mux ocelot_serdes_muxes[] = {
SERDES_MUX_SGMII(SERDES6G(1), 8, 0, 0),
SERDES_MUX_SGMII(SERDES6G(2), 10, HSIO_HW_CFG_PCIE_ENA |
HSIO_HW_CFG_DEV2G5_10_MODE, 0),
- SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, HSIO_HW_CFG_PCIE_ENA,
+ SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, 0, HSIO_HW_CFG_PCIE_ENA,
HSIO_HW_CFG_PCIE_ENA),
};
@@ -164,12 +169,17 @@ static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
unsigned int i;
int ret;
+ /* As of now only PHY_MODE_ETHERNET is supported */
+ if (mode != PHY_MODE_ETHERNET)
+ return -EOPNOTSUPP;
+
for (i = 0; i < ARRAY_SIZE(ocelot_serdes_muxes); i++) {
if (macro->idx != ocelot_serdes_muxes[i].idx ||
- mode != ocelot_serdes_muxes[i].mode)
+ mode != ocelot_serdes_muxes[i].mode ||
+ submode != ocelot_serdes_muxes[i].submode)
continue;
- if (mode != PHY_MODE_QSGMII &&
+ if (submode != PHY_INTERFACE_MODE_QSGMII &&
macro->port != ocelot_serdes_muxes[i].port)
continue;
--
2.10.5
^ permalink raw reply related
* [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
From: Grygorii Strashko @ 2018-11-20 1:24 UTC (permalink / raw)
To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko
In-Reply-To: <20181120012424.11802-1-grygorii.strashko@ti.com>
Convert mvebu-cp110-comphy PHY driver to use recently introduced
PHY_MODE_ETHERNET and phy_set_mode_ext().
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Antoine Tenart <antoine.tenart@free-electrons.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 90 ++++++++++++++-----------
2 files changed, 53 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 7a37a37..731793a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
*/
static int mvpp22_comphy_init(struct mvpp2_port *port)
{
- enum phy_mode mode;
int ret;
if (!port->comphy)
return 0;
- switch (port->phy_interface) {
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_1000BASEX:
- mode = PHY_MODE_SGMII;
- break;
- case PHY_INTERFACE_MODE_2500BASEX:
- mode = PHY_MODE_2500SGMII;
- break;
- case PHY_INTERFACE_MODE_10GKR:
- mode = PHY_MODE_10GKR;
- break;
- default:
- return -EINVAL;
- }
-
- ret = phy_set_mode(port->comphy, mode);
+ ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
+ port->phy_interface);
if (ret)
return ret;
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 79b52c3..2b4462a 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -9,6 +9,7 @@
#include <linux/iopoll.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
+#include <linux/phy.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -116,41 +117,43 @@
struct mvebu_comhy_conf {
enum phy_mode mode;
+ int submode;
unsigned lane;
unsigned port;
u32 mux;
};
-#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux) \
+#define MVEBU_COMPHY_CONF(_lane, _port, _submode, _mux) \
{ \
.lane = _lane, \
.port = _port, \
- .mode = _mode, \
+ .mode = PHY_MODE_ETHERNET, \
+ .submode = _submode, \
.mux = _mux, \
}
static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
/* lane 0 */
- MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
- MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
+ MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
+ MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
/* lane 1 */
- MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
- MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
+ MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
+ MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
/* lane 2 */
- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
/* lane 3 */
- MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
- MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
+ MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
+ MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
/* lane 4 */
- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
- MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
+ MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
/* lane 5 */
- MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
- MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
+ MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
+ MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
};
struct mvebu_comphy_priv {
@@ -163,10 +166,12 @@ struct mvebu_comphy_lane {
struct mvebu_comphy_priv *priv;
unsigned id;
enum phy_mode mode;
+ int submode;
int port;
};
-static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
+static int mvebu_comphy_get_mux(int lane, int port,
+ enum phy_mode mode, int submode)
{
int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
@@ -177,7 +182,8 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
for (i = 0; i < n; i++) {
if (mvebu_comphy_cp110_modes[i].lane == lane &&
mvebu_comphy_cp110_modes[i].port == port &&
- mvebu_comphy_cp110_modes[i].mode == mode)
+ mvebu_comphy_cp110_modes[i].mode == mode &&
+ mvebu_comphy_cp110_modes[i].submode == submode)
break;
}
@@ -187,8 +193,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
return mvebu_comphy_cp110_modes[i].mux;
}
-static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
- enum phy_mode mode)
+static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
{
struct mvebu_comphy_priv *priv = lane->priv;
u32 val;
@@ -206,14 +211,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
- if (mode == PHY_MODE_10GKR)
+ if (lane->submode == PHY_INTERFACE_MODE_10GKR)
val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
- else if (mode == PHY_MODE_2500SGMII)
+ else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
- else if (mode == PHY_MODE_SGMII)
+ else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
@@ -243,7 +248,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
/* refclk selection */
val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
- if (mode == PHY_MODE_10GKR)
+ if (lane->submode == PHY_INTERFACE_MODE_10GKR)
val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
@@ -261,8 +266,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
}
-static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
- enum phy_mode mode)
+static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
{
struct mvebu_comphy_priv *priv = lane->priv;
u32 val;
@@ -303,13 +307,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
return 0;
}
-static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
+static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
{
struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
struct mvebu_comphy_priv *priv = lane->priv;
u32 val;
- mvebu_comphy_ethernet_init_reset(lane, mode);
+ mvebu_comphy_ethernet_init_reset(lane);
val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
@@ -330,7 +334,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
- return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
+ return mvebu_comphy_init_plls(lane);
}
static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
@@ -339,7 +343,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
struct mvebu_comphy_priv *priv = lane->priv;
u32 val;
- mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
+ mvebu_comphy_ethernet_init_reset(lane);
val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
@@ -469,7 +473,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
- return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
+ return mvebu_comphy_init_plls(lane);
}
static int mvebu_comphy_power_on(struct phy *phy)
@@ -479,7 +483,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
int ret, mux;
u32 val;
- mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
+ mux = mvebu_comphy_get_mux(lane->id, lane->port,
+ lane->mode, lane->submode);
if (mux < 0)
return -ENOTSUPP;
@@ -492,12 +497,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
- switch (lane->mode) {
- case PHY_MODE_SGMII:
- case PHY_MODE_2500SGMII:
- ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
+ switch (lane->submode) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ ret = mvebu_comphy_set_mode_sgmii(phy);
break;
- case PHY_MODE_10GKR:
+ case PHY_INTERFACE_MODE_10GKR:
ret = mvebu_comphy_set_mode_10gkr(phy);
break;
default:
@@ -517,10 +522,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
{
struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
- if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
+ if (mode != PHY_MODE_ETHERNET)
+ return -EINVAL;
+
+ if (submode == PHY_INTERFACE_MODE_1000BASEX)
+ submode = PHY_INTERFACE_MODE_SGMII;
+
+ if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
return -EINVAL;
lane->mode = mode;
+ lane->submode = submode;
return 0;
}
--
2.10.5
^ permalink raw reply related
* [PATCH v3 5/5] phy: core: clean up unused ethernet specific phy modes
From: Grygorii Strashko @ 2018-11-20 1:24 UTC (permalink / raw)
To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko
In-Reply-To: <20181120012424.11802-1-grygorii.strashko@ti.com>
After recent changes PHY_MODE_SGMII, PHY_MODE_2500SGMII, PHY_MODE_QSGMII,
PHY_MODE_10GKR are not used any more and can be removed. Hence - remove
them.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
include/linux/phy/phy.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 02c9ef0..79da05a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -35,10 +35,6 @@ enum phy_mode {
PHY_MODE_USB_DEVICE_HS,
PHY_MODE_USB_DEVICE_SS,
PHY_MODE_USB_OTG,
- PHY_MODE_SGMII,
- PHY_MODE_2500SGMII,
- PHY_MODE_QSGMII,
- PHY_MODE_10GKR,
PHY_MODE_UFS_HS_A,
PHY_MODE_UFS_HS_B,
PHY_MODE_PCIE,
--
2.10.5
^ permalink raw reply related
* Re: [PATCH net-next,v2 05/12] cls_flower: add statistics retrieval infrastructure and use it
From: Jiri Pirko @ 2018-11-19 15:04 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz
In-Reply-To: <20181119144850.wfl22tlcvdlkddl4@salvia>
Mon, Nov 19, 2018 at 03:48:50PM CET, pablo@netfilter.org wrote:
>On Mon, Nov 19, 2018 at 02:57:05PM +0100, Jiri Pirko wrote:
>> Mon, Nov 19, 2018 at 01:15:12AM CET, pablo@netfilter.org wrote:
>[...]
>> >diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>> >index 7d7aefa5fcd2..7f9a8d5ca945 100644
>> >--- a/include/net/pkt_cls.h
>> >+++ b/include/net/pkt_cls.h
>> >@@ -758,6 +758,12 @@ enum tc_fl_command {
>> > TC_CLSFLOWER_TMPLT_DESTROY,
>> > };
>> >
>> >+struct tc_cls_flower_stats {
>> >+ u64 pkts;
>> >+ u64 bytes;
>> >+ u64 lastused;
>> >+};
>> >+
>> > struct tc_cls_flower_offload {
>> > struct tc_cls_common_offload common;
>> > enum tc_fl_command command;
>> >@@ -765,6 +771,7 @@ struct tc_cls_flower_offload {
>> > struct flow_rule rule;
>> > struct tcf_exts *exts;
>> > u32 classid;
>> >+ struct tc_cls_flower_stats stats;
>> > };
>> >
>> > static inline struct flow_rule *
>> >@@ -773,6 +780,14 @@ tc_cls_flower_offload_flow_rule(struct tc_cls_flower_offload *tc_flow_cmd)
>> > return &tc_flow_cmd->rule;
>> > }
>> >
>> >+static inline void tc_cls_flower_stats_update(struct tc_cls_flower_offload *cls_flower,
>> >+ u64 pkts, u64 bytes, u64 lastused)
>> >+{
>> >+ cls_flower->stats.pkts = pkts;
>> >+ cls_flower->stats.bytes = bytes;
>> >+ cls_flower->stats.lastused = lastused;
>>
>> Why do you need to store the values here in struct tc_cls_flower_offload?
>> Why don't you just call tcf_exts_stats_update()? Basically,
>> tc_cls_flower_stats_update() should be just wrapper around
>> tcf_exts_stats_update() so that drivers wouldn't use ->exts directly, as
>> you will remove them in follow-up patches, no?
>
>Patch 07/12 stops exposing tc action exts to drivers, so we need a
>structure (struct tc_cls_flower_stats) to convey this statistics back
>to the cls_flower frontend.
Hmm, shouldn't these stats be rather flow_rule related than flower
related?
^ permalink raw reply
* Re: [PATCH v2 RESEND 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Aaron Lu @ 2018-11-20 1:43 UTC (permalink / raw)
To: Tariq Toukan
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Andrew Morton, Paweł Staszewski,
Jesper Dangaard Brouer, Eric Dumazet, Ilias Apalodimas,
Yoel Caspersen, Mel Gorman, Saeed Mahameed, Michal Hocko,
Vlastimil Babka, Dave Hansen, Alexander Duyck, Ian Kumlien
In-Reply-To: <be048469-d29d-e700-4858-4a5263a50eb6@mellanox.com>
On Mon, Nov 19, 2018 at 03:00:53PM +0000, Tariq Toukan wrote:
>
>
> On 19/11/2018 3:48 PM, Aaron Lu wrote:
> > page_frag_free() calls __free_pages_ok() to free the page back to
> > Buddy. This is OK for high order page, but for order-0 pages, it
> > misses the optimization opportunity of using Per-Cpu-Pages and can
> > cause zone lock contention when called frequently.
> >
> > Paweł Staszewski recently shared his result of 'how Linux kernel
> > handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> > found the lock contention comes from page allocator:
> >
> > mlx5e_poll_tx_cq
> > |
> > --16.34%--napi_consume_skb
> > |
> > |--12.65%--__free_pages_ok
> > | |
> > | --11.86%--free_one_page
> > | |
> > | |--10.10%--queued_spin_lock_slowpath
> > | |
> > | --0.65%--_raw_spin_lock
> > |
> > |--1.55%--page_frag_free
> > |
> > --1.44%--skb_release_data
> >
> > Jesper explained how it happened: mlx5 driver RX-page recycle
> > mechanism is not effective in this workload and pages have to go
> > through the page allocator. The lock contention happens during
> > mlx5 DMA TX completion cycle. And the page allocator cannot keep
> > up at these speeds.[2]
> >
> > I thought that __free_pages_ok() are mostly freeing high order
> > pages and thought this is an lock contention for high order pages
> > but Jesper explained in detail that __free_pages_ok() here are
> > actually freeing order-0 pages because mlx5 is using order-0 pages
> > to satisfy its page pool allocation request.[3]
> >
> > The free path as pointed out by Jesper is:
> > skb_free_head()
> > -> skb_free_frag()
> > -> page_frag_free()
> > And the pages being freed on this path are order-0 pages.
> >
> > Fix this by doing similar things as in __page_frag_cache_drain() -
> > send the being freed page to PCP if it's an order-0 page, or
> > directly to Buddy if it is a high order page.
> >
> > With this change, Paweł hasn't noticed lock contention yet in
> > his workload and Jesper has noticed a 7% performance improvement
> > using a micro benchmark and lock contention is gone. Ilias' test
> > on a 'low' speed 1Gbit interface on an cortex-a53 shows ~11%
> > performance boost testing with 64byte packets and __free_pages_ok()
> > disappeared from perf top.
> >
> > [1]: https://www.spinics.net/lists/netdev/msg531362.html
> > [2]: https://www.spinics.net/lists/netdev/msg531421.html
> > [3]: https://www.spinics.net/lists/netdev/msg531556.html
> >
> > Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> > Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > Acked-by: Mel Gorman <mgorman@techsingularity.net>
> > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > Acked-by: Tariq Toukan <tariqt@mellanox.com
>
> missing '>' sign in my email tag.
Sorry about that, will fix this and resend.
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > ---
^ permalink raw reply
* [PATCH v2 RESEND update 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Aaron Lu @ 2018-11-20 1:45 UTC (permalink / raw)
To: linux-mm, linux-kernel, netdev
Cc: Andrew Morton, Paweł Staszewski, Jesper Dangaard Brouer,
Eric Dumazet, Tariq Toukan, Ilias Apalodimas, Yoel Caspersen,
Mel Gorman, Saeed Mahameed, Michal Hocko, Vlastimil Babka,
Dave Hansen, Alexander Duyck, Ian Kumlien
In-Reply-To: <20181119134834.17765-2-aaron.lu@intel.com>
page_frag_free() calls __free_pages_ok() to free the page back to
Buddy. This is OK for high order page, but for order-0 pages, it
misses the optimization opportunity of using Per-Cpu-Pages and can
cause zone lock contention when called frequently.
Paweł Staszewski recently shared his result of 'how Linux kernel
handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
found the lock contention comes from page allocator:
mlx5e_poll_tx_cq
|
--16.34%--napi_consume_skb
|
|--12.65%--__free_pages_ok
| |
| --11.86%--free_one_page
| |
| |--10.10%--queued_spin_lock_slowpath
| |
| --0.65%--_raw_spin_lock
|
|--1.55%--page_frag_free
|
--1.44%--skb_release_data
Jesper explained how it happened: mlx5 driver RX-page recycle
mechanism is not effective in this workload and pages have to go
through the page allocator. The lock contention happens during
mlx5 DMA TX completion cycle. And the page allocator cannot keep
up at these speeds.[2]
I thought that __free_pages_ok() are mostly freeing high order
pages and thought this is an lock contention for high order pages
but Jesper explained in detail that __free_pages_ok() here are
actually freeing order-0 pages because mlx5 is using order-0 pages
to satisfy its page pool allocation request.[3]
The free path as pointed out by Jesper is:
skb_free_head()
-> skb_free_frag()
-> page_frag_free()
And the pages being freed on this path are order-0 pages.
Fix this by doing similar things as in __page_frag_cache_drain() -
send the being freed page to PCP if it's an order-0 page, or
directly to Buddy if it is a high order page.
With this change, Paweł hasn't noticed lock contention yet in
his workload and Jesper has noticed a 7% performance improvement
using a micro benchmark and lock contention is gone. Ilias' test
on a 'low' speed 1Gbit interface on an cortex-a53 shows ~11%
performance boost testing with 64byte packets and __free_pages_ok()
disappeared from perf top.
[1]: https://www.spinics.net/lists/netdev/msg531362.html
[2]: https://www.spinics.net/lists/netdev/msg531421.html
[3]: https://www.spinics.net/lists/netdev/msg531556.html
Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Acked-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
update: fix Tariq's email tag.
mm/page_alloc.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 421c5b652708..8f8c6b33b637 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4677,8 +4677,14 @@ void page_frag_free(void *addr)
{
struct page *page = virt_to_head_page(addr);
- if (unlikely(put_page_testzero(page)))
- __free_pages_ok(page, compound_order(page));
+ if (unlikely(put_page_testzero(page))) {
+ unsigned int order = compound_order(page);
+
+ if (order == 0)
+ free_unref_page(page);
+ else
+ __free_pages_ok(page, order);
+ }
}
EXPORT_SYMBOL(page_frag_free);
--
2.17.2
^ permalink raw reply related
* Re: [PATCH net-next] net: hns3: add common validation in hclge_dcb
From: Yunsheng Lin @ 2018-11-20 1:50 UTC (permalink / raw)
To: Tan Xiaojun, yisen.zhuang, salil.mehta, davem, lipeng321,
shenjian15, tanhuazhong, liangfuyun1
Cc: netdev, linux-kernel, nhorman
In-Reply-To: <1542632535-18210-1-git-send-email-tanxiaojun@huawei.com>
On 2018/11/19 21:02, Tan Xiaojun wrote:
> From: Yunsheng Lin <linyunsheng@huawei.com>
>
> Before setting tm related configuration to hardware, driver
> needs to check the configuration provided by user is valid.
> Currently hclge_ieee_setets and hclge_setup_tc both implement
> their own checking, which has a lot in common.
>
> This patch addes hclge_dcb_common_validate to do the common
> checking. The checking in hclge_tm_prio_tc_info_update
> and hclge_tm_schd_info_update is unnecessary now, so change
> the return type to void, which removes the need to do error
> handling when one of the checking fails.
>
> Also, ets->prio_tc is indexed by user prio and ets->tc_tsa is
> indexed by tc num, so this patch changes them to use different
> index.
+cc Neil
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
^ permalink raw reply
* BPF probe namespacing
From: Peter Parkanyi @ 2018-11-19 15:29 UTC (permalink / raw)
To: netdev; +Cc: alexei.starovoitov
Hi,
At LPC I raised the observation that currently it doesn't seem
feasible to insert a BPF probe from within a container that sees
events happening outside of the container, while it is possible to
insert a kernel module.
It was suggested that this is not the case, and things should just work.
I wanted to get a minimal reproduction of what I've seen in Docker
containers, so if somebody could take a look, I'd appreciate any
comments on the right way of doing this.
The kprobe in question:
https://github.com/redsift/ingraind/blob/master/bpf/file.c
BCC's libbpf does the attach:
https://github.com/iovisor/bcc/blob/master/src/cc/libbpf.c#L829
# Steps to reproduce
Build the bpf module & loader (generic binary targeting 4.16/x86_64 at
https://123-130035428-gh.circle-artifacts.com/0/ingraind ):
1. get a rust toolchain, musl-dev
2. git clone https://github.com/redsift/ingraind; cd ingraind;
KERNEL_SOURCE=<path to src> cargo +nightly build
--target=x86_64-unknown-linux-musl --release
## Run the BPF module without a sandbox
3. echo >config <<EOF
[[probe]]
pipelines = ["console"]
[probe.config]
type = "Files"
monitor_dirs = ["/"]not
[pipeline.console.config]
backend = "Console"
EOF
4. sudo ./target/x86_64-unknown-linux-musl/release/ingraind config
5. You can see that all VFS operations from the host are listed.
6. Kill the process C-c
### Expectation
I get system-wide filesystem events through the VFS
### Reality
Meets the expectation
## Run BPF module from chroot
1. mkdir -p test/proc test/sys; cp
/target/x86_64-unknown-linux-musl/release/ingraind config test
2. sudo mount -t sysfs sys test/sys; sudo mount -t debugfs none
test/sys/kernel/debug
3. sudo chroot ./test /ingraind /config
### Expectation
I see system-wide events, just like without chroot.
### Reality
I don't see events firing at all.
If you compile the code at
https://gist.github.com/rsdy/bfe45ebae7354217e7242c8bf10df274
statically and run it inside the chroot while ingraind is running
chrooted, the kprobe will fire both read and write events from within
the mount namespace.
Thanks,
Peter
--
Red Sift is the power behind OnDMARC
You can find us at 20 Air Street,
4th Floor at Wayra, London, W1B 5AN
Red Sift is a limited company
registered in England and Wales. Registered number: 09240956. Registered
office: Kemp House, 152 City Road, London, EC1V 2NX.
^ permalink raw reply
* Re: [PATCH v3] net: Add trace events for all receive exit points
From: Geneviève Bastien @ 2018-11-19 15:27 UTC (permalink / raw)
To: David Miller, mathieu.desnoyers; +Cc: netdev, rostedt, mingo
In-Reply-To: <20181117.221921.901866732366630755.davem@davemloft.net>
On 2018-11-18 1:19 a.m., David Miller wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Date: Sat, 17 Nov 2018 13:27:29 -0500 (EST)
>
>> I see two possible solutions:
>>
>> 1) Remove the "skb" argument from the sbk_exit tracepoints completely.
>> Anyway, I think it's not really needed for analysis purposes because
>> we can link the "entry" with the associated "exit" using the thread ID
>> executing those tracepoints. (Genevi�ve, would that work for your
>> analyses ?)
>>
>> 2) Move the skb_exit tracepoints before freeing the skb pointer. My
>> concern here is that the instrumentation may become much uglier than
>> the currently proposed patch. (I have not looked at the specifics
>> though, so I may be wrong.)
>>
>> Do you have a preference between those two approaches, or perhaps you
>> have an alternative solution in mind ?
> I wonder how other situations handle this.
>
> About #2, if you put the tracepoint beforehand you can't log the
> 'ret' value. So at least in that regard I prefer #1.
>
> If tracepoints generally handle this by matching up the thread
> ID, then definitely that's how we should do it here too instead
> of trying to use the SKB pointer for this purpose.
I would go for #1 too, the "skb" is not used to match entry/exit, it is more the context in which they appear (thread, softirq, etc). And I did indeed get seg faults on my first attempt when I tried to use the existing templates.
There's just the list tracepoint that would now log nothing, so there's no point looping through the list.
^ permalink raw reply
* Re: [PATCH 1/4] bpf: account for freed JIT allocations in arch code
From: Ard Biesheuvel @ 2018-11-19 15:37 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Linux Kernel Mailing List, Alexei Starovoitov, Rick Edgecombe,
Eric Dumazet, Jann Horn, Kees Cook, Jessica Yu, Arnd Bergmann,
Catalin Marinas, Will Deacon, Mark Rutland, Ralf Baechle,
Paul Burton, James Hogan, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, David S. Miller, linux-arm-kernel, linu
In-Reply-To: <b37c3411-7950-dcb3-f0c3-6d9f589d36ab@iogearbox.net>
On Mon, 19 Nov 2018 at 02:37, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 11/17/2018 07:57 PM, Ard Biesheuvel wrote:
> > Commit ede95a63b5e84 ("bpf: add bpf_jit_limit knob to restrict unpriv
> > allocations") added a call to bpf_jit_uncharge_modmem() to the routine
> > bpf_jit_binary_free() which is called from the __weak bpf_jit_free().
> > This function is overridden by arches, some of which do not call
> > bpf_jit_binary_free() to release the memory, and so the released
> > memory is not accounted for, potentially leading to spurious allocation
> > failures.
> >
> > So replace the direct calls to module_memfree() in the arch code with
> > calls to bpf_jit_binary_free().
>
> Sorry but this patch is completely buggy, and above description on the
> accounting incorrect as well. Looks like this patch was not tested at all.
>
My apologies. I went off into the weeds a bit looking at different
versions for 32-bit and 64-bit on different architectures. So indeed,
this patch should be dropped.
> The below cBPF JITs that use module_memfree() which you replace with
> bpf_jit_binary_free() are using module_alloc() internally to get the JIT
> image buffer ...
>
Indeed. So would you prefer for arm64 to override bpf_jit_free() in
its entirety, and not call bpf_jit_binary_free() but simply call
bpf_jit_uncharge_modmem() and vfree() directly? It's either that, or
we'd have to untangle this a bit, to avoid having one __weak function
on top of the other just so other arches can replace the
module_memfree() call in bpf_jit_binary_free() with vfree() (which
amount to the same thing on arm64 anyway)
^ permalink raw reply
* 答复: [PATCH 1/4] net-next/hinic:replace multiply and division operators
From: xuechaojing @ 2018-11-20 2:21 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, wulike (A),
chiqijun, Wangfengying, Quhuichun (Tony), Luoshaokai (luoshaokai),
xuechaojing
In-Reply-To: <20181119.150142.777293649598457239.davem@davemloft.net>
Hi:
This wqebb size is 32 in rx and in tx is 64, so the value is a power of two.
Thank you
-----邮件原件-----
发件人: David Miller [mailto:davem@davemloft.net]
发送时间: 2018年11月20日 7:02
收件人: xuechaojing <xuechaojing@huawei.com>
抄送: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; wulike (A) <wulike1@huawei.com>; chiqijun <chiqijun@huawei.com>; Wangfengying <fy.wang@huawei.com>; Quhuichun (Tony) <tony.qu@huawei.com>; Luoshaokai (luoshaokai) <luoshaokai@huawei.com>
主题: Re: [PATCH 1/4] net-next/hinic:replace multiply and division operators
From: Xue Chaojing <xuechaojing@huawei.com>
Date: Mon, 19 Nov 2018 06:12:31 +0000
> @@ -530,7 +536,9 @@ int hinic_wq_allocate(struct hinic_wqs *wqs, struct hinic_wq *wq,
> return -EINVAL;
> }
>
> - num_wqebbs_per_page = ALIGN(wq_page_size, wqebb_size) / wqebb_size;
> + wqebb_size_shift = ilog2(wqebb_size);
You now have introduced the assumption that these various sizes are a power of two.
You should check for this, either at compile time or at run time, in order to avoid surprises and hard to debug issues in the future.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 12/12] qede: use ethtool_rx_flow_rule() to remove duplicated parser code
From: Jiri Pirko @ 2018-11-19 16:00 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr, ogerlitz
In-Reply-To: <20181119001519.12124-13-pablo@netfilter.org>
Mon, Nov 19, 2018 at 01:15:19AM CET, pablo@netfilter.org wrote:
>The qede driver supports for ethtool_rx_flow_spec and flower, both
>codebases look very similar.
>
>This patch uses the ethtool_rx_flow_rule() infrastructure to remove the
>duplicated ethtool_rx_flow_spec parser and consolidate ACL offload
>support around the flow_rule infrastructure.
>
>Furthermore, more code can be consolidated by merging
>qede_add_cls_rule() and qede_add_tc_flower_fltr(), these two functions
>also look very similar.
>
>This driver currently provides simple ACL support, such as 5-tuple
>matching, drop policy and queue to CPU.
>
>Drivers that support more features can benefit from this infrastructure
>to save even more redundant codebase.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
>Note that, after this patch, qede_add_cls_rule() and
>qede_add_tc_flower_fltr() can be also consolidated since their code is
>redundant.
>
> drivers/net/ethernet/qlogic/qede/qede_filter.c | 246 ++++++-------------------
> 1 file changed, 53 insertions(+), 193 deletions(-)
>
>diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
>index aca302c3261b..f82b26ba8f80 100644
>--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
>+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
>@@ -1578,30 +1578,6 @@ static void qede_flow_build_ipv6_hdr(struct qede_arfs_tuple *t,
> ports[1] = t->dst_port;
> }
>
>-/* Validate fields which are set and not accepted by the driver */
>-static int qede_flow_spec_validate_unused(struct qede_dev *edev,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- if (fs->flow_type & FLOW_MAC_EXT) {
>- DP_INFO(edev, "Don't support MAC extensions\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->flow_type & FLOW_EXT) &&
>- (fs->h_ext.vlan_etype || fs->h_ext.vlan_tci)) {
>- DP_INFO(edev, "Don't support vlan-based classification\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->flow_type & FLOW_EXT) &&
>- (fs->h_ext.data[0] || fs->h_ext.data[1])) {
>- DP_INFO(edev, "Don't support user defined data\n");
>- return -EOPNOTSUPP;
>- }
>-
>- return 0;
>-}
>-
> static int qede_set_v4_tuple_to_profile(struct qede_dev *edev,
> struct qede_arfs_tuple *t)
> {
>@@ -1665,132 +1641,6 @@ static int qede_set_v6_tuple_to_profile(struct qede_dev *edev,
> return 0;
> }
>
>-static int qede_flow_spec_to_tuple_ipv4_common(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- if ((fs->h_u.tcp_ip4_spec.ip4src &
>- fs->m_u.tcp_ip4_spec.ip4src) != fs->h_u.tcp_ip4_spec.ip4src) {
>- DP_INFO(edev, "Don't support IP-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->h_u.tcp_ip4_spec.ip4dst &
>- fs->m_u.tcp_ip4_spec.ip4dst) != fs->h_u.tcp_ip4_spec.ip4dst) {
>- DP_INFO(edev, "Don't support IP-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->h_u.tcp_ip4_spec.psrc &
>- fs->m_u.tcp_ip4_spec.psrc) != fs->h_u.tcp_ip4_spec.psrc) {
>- DP_INFO(edev, "Don't support port-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->h_u.tcp_ip4_spec.pdst &
>- fs->m_u.tcp_ip4_spec.pdst) != fs->h_u.tcp_ip4_spec.pdst) {
>- DP_INFO(edev, "Don't support port-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if (fs->h_u.tcp_ip4_spec.tos) {
>- DP_INFO(edev, "Don't support tos\n");
>- return -EOPNOTSUPP;
>- }
>-
>- t->eth_proto = htons(ETH_P_IP);
>- t->src_ipv4 = fs->h_u.tcp_ip4_spec.ip4src;
>- t->dst_ipv4 = fs->h_u.tcp_ip4_spec.ip4dst;
>- t->src_port = fs->h_u.tcp_ip4_spec.psrc;
>- t->dst_port = fs->h_u.tcp_ip4_spec.pdst;
>-
>- return qede_set_v4_tuple_to_profile(edev, t);
>-}
>-
>-static int qede_flow_spec_to_tuple_tcpv4(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- t->ip_proto = IPPROTO_TCP;
>-
>- if (qede_flow_spec_to_tuple_ipv4_common(edev, t, fs))
>- return -EINVAL;
>-
>- return 0;
>-}
>-
>-static int qede_flow_spec_to_tuple_udpv4(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- t->ip_proto = IPPROTO_UDP;
>-
>- if (qede_flow_spec_to_tuple_ipv4_common(edev, t, fs))
>- return -EINVAL;
>-
>- return 0;
>-}
>-
>-static int qede_flow_spec_to_tuple_ipv6_common(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- struct in6_addr zero_addr;
>-
>- memset(&zero_addr, 0, sizeof(zero_addr));
>-
>- if ((fs->h_u.tcp_ip6_spec.psrc &
>- fs->m_u.tcp_ip6_spec.psrc) != fs->h_u.tcp_ip6_spec.psrc) {
>- DP_INFO(edev, "Don't support port-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if ((fs->h_u.tcp_ip6_spec.pdst &
>- fs->m_u.tcp_ip6_spec.pdst) != fs->h_u.tcp_ip6_spec.pdst) {
>- DP_INFO(edev, "Don't support port-masks\n");
>- return -EOPNOTSUPP;
>- }
>-
>- if (fs->h_u.tcp_ip6_spec.tclass) {
>- DP_INFO(edev, "Don't support tclass\n");
>- return -EOPNOTSUPP;
>- }
>-
>- t->eth_proto = htons(ETH_P_IPV6);
>- memcpy(&t->src_ipv6, &fs->h_u.tcp_ip6_spec.ip6src,
>- sizeof(struct in6_addr));
>- memcpy(&t->dst_ipv6, &fs->h_u.tcp_ip6_spec.ip6dst,
>- sizeof(struct in6_addr));
>- t->src_port = fs->h_u.tcp_ip6_spec.psrc;
>- t->dst_port = fs->h_u.tcp_ip6_spec.pdst;
>-
>- return qede_set_v6_tuple_to_profile(edev, t, &zero_addr);
>-}
>-
>-static int qede_flow_spec_to_tuple_tcpv6(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- t->ip_proto = IPPROTO_TCP;
>-
>- if (qede_flow_spec_to_tuple_ipv6_common(edev, t, fs))
>- return -EINVAL;
>-
>- return 0;
>-}
>-
>-static int qede_flow_spec_to_tuple_udpv6(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>-{
>- t->ip_proto = IPPROTO_UDP;
>-
>- if (qede_flow_spec_to_tuple_ipv6_common(edev, t, fs))
>- return -EINVAL;
>-
>- return 0;
>-}
>-
> /* Must be called while qede lock is held */
> static struct qede_arfs_fltr_node *
> qede_flow_find_fltr(struct qede_dev *edev, struct qede_arfs_tuple *t)
>@@ -1875,25 +1725,32 @@ static int qede_parse_actions(struct qede_dev *edev,
> struct flow_action *flow_action)
> {
> const struct flow_action_key *act;
>- int rc = -EINVAL, num_act = 0, i;
>- bool is_drop = false;
>+ int i;
>
> if (!flow_action_has_keys(flow_action)) {
>- DP_NOTICE(edev, "No tc actions received\n");
>- return rc;
>+ DP_NOTICE(edev, "No actions received\n");
>+ return -EINVAL;
> }
>
> flow_action_for_each(i, act, flow_action) {
>- num_act++;
>+ switch (act->id) {
>+ case FLOW_ACTION_KEY_DROP:
>+ break;
>+ case FLOW_ACTION_KEY_QUEUE:
>+ if (ethtool_get_flow_spec_ring_vf(act->queue_index))
>+ break;
>
>- if (act->id == FLOW_ACTION_KEY_DROP)
>- is_drop = true;
>+ if (act->queue_index >= QEDE_RSS_COUNT(edev)) {
>+ DP_INFO(edev, "Queue out-of-bounds\n");
>+ return -EINVAL;
>+ }
>+ break;
>+ default:
>+ return -EINVAL;
>+ }
> }
>
>- if (num_act == 1 && is_drop)
>- return 0;
>-
>- return rc;
>+ return 0;
> }
>
> static int
>@@ -2147,16 +2004,17 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
> }
>
> static int qede_flow_spec_validate(struct qede_dev *edev,
>- struct ethtool_rx_flow_spec *fs,
>- struct qede_arfs_tuple *t)
>+ struct flow_action *flow_action,
>+ struct qede_arfs_tuple *t,
>+ __u32 location)
> {
>- if (fs->location >= QEDE_RFS_MAX_FLTR) {
>+ if (location >= QEDE_RFS_MAX_FLTR) {
> DP_INFO(edev, "Location out-of-bounds\n");
> return -EINVAL;
> }
>
> /* Check location isn't already in use */
>- if (test_bit(fs->location, edev->arfs->arfs_fltr_bmap)) {
>+ if (test_bit(location, edev->arfs->arfs_fltr_bmap)) {
> DP_INFO(edev, "Location already in use\n");
> return -EINVAL;
> }
>@@ -2170,46 +2028,53 @@ static int qede_flow_spec_validate(struct qede_dev *edev,
> return -EINVAL;
> }
>
>- /* If drop requested then no need to validate other data */
>- if (fs->ring_cookie == RX_CLS_FLOW_DISC)
>- return 0;
>-
>- if (ethtool_get_flow_spec_ring_vf(fs->ring_cookie))
>- return 0;
>-
>- if (fs->ring_cookie >= QEDE_RSS_COUNT(edev)) {
>- DP_INFO(edev, "Queue out-of-bounds\n");
>+ if (qede_parse_actions(edev, flow_action))
> return -EINVAL;
>- }
>
> return 0;
> }
>
>-static int qede_flow_spec_to_tuple(struct qede_dev *edev,
>- struct qede_arfs_tuple *t,
>- struct ethtool_rx_flow_spec *fs)
>+static int qede_flow_spec_to_rule(struct qede_dev *edev,
>+ struct qede_arfs_tuple *t,
>+ struct ethtool_rx_flow_spec *fs)
> {
>- memset(t, 0, sizeof(*t));
>-
>- if (qede_flow_spec_validate_unused(edev, fs))
>- return -EOPNOTSUPP;
>+ struct tc_cls_flower_offload f = {};
>+ struct flow_rule *flow_rule;
>+ __be16 proto;
>+ int err = 0;
>
> switch ((fs->flow_type & ~FLOW_EXT)) {
> case TCP_V4_FLOW:
>- return qede_flow_spec_to_tuple_tcpv4(edev, t, fs);
> case UDP_V4_FLOW:
>- return qede_flow_spec_to_tuple_udpv4(edev, t, fs);
>+ proto = htons(ETH_P_IP);
>+ break;
> case TCP_V6_FLOW:
>- return qede_flow_spec_to_tuple_tcpv6(edev, t, fs);
> case UDP_V6_FLOW:
>- return qede_flow_spec_to_tuple_udpv6(edev, t, fs);
>+ proto = htons(ETH_P_IPV6);
>+ break;
> default:
> DP_VERBOSE(edev, NETIF_MSG_IFUP,
> "Can't support flow of type %08x\n", fs->flow_type);
> return -EOPNOTSUPP;
> }
>
>- return 0;
>+ flow_rule = ethtool_rx_flow_rule(fs);
>+ if (!flow_rule)
>+ return -ENOMEM;
>+
>+ f.rule = *flow_rule;
This does not look right. I undersntand that you want to use the same
driver code to parse same struct coming either from tc-flower or
ethtool. That is why you introduced flow_rule as a intermediate layer.
However, here, you use struct that is very tc-flower specific. Common
parser should work on struct flow_rule.
qede_parse_flower_attr() should accept struct flow_rule * and should be
renamed to something like qede_parse_flow_rule()
>+
>+ if (qede_parse_flower_attr(edev, proto, &f, t)) {
>+ err = -EINVAL;
>+ goto err_out;
>+ }
>+
>+ /* Make sure location is valid and filter isn't already set */
>+ err = qede_flow_spec_validate(edev, &f.rule.action, t, fs->location);
>+err_out:
>+ ethtool_rx_flow_rule_free(flow_rule);
>+ return err;
>+
> }
>
> int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info)
>@@ -2227,12 +2092,7 @@ int qede_add_cls_rule(struct qede_dev *edev, struct ethtool_rxnfc *info)
> }
>
> /* Translate the flow specification into something fittign our DB */
>- rc = qede_flow_spec_to_tuple(edev, &t, fsp);
>- if (rc)
>- goto unlock;
>-
>- /* Make sure location is valid and filter isn't already set */
>- rc = qede_flow_spec_validate(edev, fsp, &t);
>+ rc = qede_flow_spec_to_rule(edev, &t, fsp);
> if (rc)
> goto unlock;
>
>--
>2.11.0
>
^ permalink raw reply
* Re: [PATCH net-next] net: hns3: add common validation in hclge_dcb
From: David Miller @ 2018-11-20 2:32 UTC (permalink / raw)
To: tanxiaojun
Cc: yisen.zhuang, salil.mehta, lipeng321, linyunsheng, shenjian15,
tanhuazhong, liangfuyun1, netdev, linux-kernel
In-Reply-To: <1542632535-18210-1-git-send-email-tanxiaojun@huawei.com>
From: Tan Xiaojun <tanxiaojun@huawei.com>
Date: Mon, 19 Nov 2018 21:02:15 +0800
> From: Yunsheng Lin <linyunsheng@huawei.com>
>
> Before setting tm related configuration to hardware, driver
> needs to check the configuration provided by user is valid.
> Currently hclge_ieee_setets and hclge_setup_tc both implement
> their own checking, which has a lot in common.
>
> This patch addes hclge_dcb_common_validate to do the common
> checking. The checking in hclge_tm_prio_tc_info_update
> and hclge_tm_schd_info_update is unnecessary now, so change
> the return type to void, which removes the need to do error
> handling when one of the checking fails.
>
> Also, ets->prio_tc is indexed by user prio and ets->tc_tsa is
> indexed by tc num, so this patch changes them to use different
> index.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
Looks good, applied, thanks!
^ permalink raw reply
* [PATCH net-next 00/18] selftests: Add tests for VXLAN at an 802.1d bridge
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
Petr says:
This patchset adds several tests for VXLAN attached to an 802.1d bridge
and fixes a related bug.
First patch #1 fixes a bug in propagating SKB already-forwarded marks
over veth to bridges, where they are irrelevant. This bug causes the
vxlan_bridge_1d test suite from this patchset to fail as the packets
aren't forwarded by br2.
In patches #2 and #3, lib.sh is extended to support network namespaces.
The use of namespaces is necessitated by VXLAN, which allows only one
VXLAN device with a given VNI per namespace. Thus to host full topology
on a single box for selftests, the "remote" endpoints need to be in
namespaces.
In patches #4-#6, lib.sh is extended in other ways to facilitate the
following patches.
In patches #7-#15, first the skeleton, and later the generic tests
themselves are added.
Patch #16 then adds another test that serves as a wrapper around the
previous one, and runs it with a non-default port number.
Patches #17 and #18 add mlxsw-specific tests. About those, Ido writes:
The first test creates various configurations with regards to the VxLAN
and bridge devices and makes sure the driver correctly forbids
unsupported configuration and permits supported ones. It also verifies
that the driver correctly sets the offload indication on FDB entries and
the local route used for VxLAN decapsulation.
The second test verifies that the driver correctly configures the singly
linked list used to flood BUM traffic and that traffic is flooded as
expected.
Ido Schimmel (2):
selftests: mlxsw: Add a test for VxLAN configuration
selftests: mlxsw: Add a test for VxLAN flooding
Petr Machata (16):
net: skb_scrub_packet(): Scrub offload_fwd_mark
selftests: forwarding: lib: Support NUM_NETIFS of 0
selftests: forwarding: lib: Add in_ns()
selftests: forwarding: ping{6,}_test(): Add description argument
selftests: forwarding: ping{6,}_do(): Allow passing ping arguments
selftests: forwarding: lib: Add link_stats_rx_errors_get()
selftests: forwarding: Add a skeleton of vxlan_bridge_1d
selftests: forwarding: vxlan_bridge_1d: Add ping test
selftests: forwarding: vxlan_bridge_1d: Add flood test
selftests: forwarding: vxlan_bridge_1d: Add unicast test
selftests: forwarding: vxlan_bridge_1d: Reconfigure & rerun tests
selftests: forwarding: vxlan_bridge_1d: Add a TTL test
selftests: forwarding: vxlan_bridge_1d: Add a TOS test
selftests: forwarding: vxlan_bridge_1d: Add an ECN encap test
selftests: forwarding: vxlan_bridge_1d: Add an ECN decap test
selftests: forwarding: vxlan_bridge_1d_port_8472: New test
net/core/skbuff.c | 5 +
.../selftests/drivers/net/mlxsw/vxlan.sh | 664 +++++++++++++++++
.../drivers/net/mlxsw/vxlan_flooding.sh | 309 ++++++++
tools/testing/selftests/net/forwarding/lib.sh | 42 +-
.../net/forwarding/vxlan_bridge_1d.sh | 678 ++++++++++++++++++
.../forwarding/vxlan_bridge_1d_port_8472.sh | 10 +
6 files changed, 1700 insertions(+), 8 deletions(-)
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/vxlan.sh
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh
create mode 100755 tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
create mode 100755 tools/testing/selftests/net/forwarding/vxlan_bridge_1d_port_8472.sh
--
2.19.1
^ permalink raw reply
* [PATCH net-next 01/18] net: skb_scrub_packet(): Scrub offload_fwd_mark
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
When a packet is trapped and the corresponding SKB marked as
already-forwarded, it retains this marking even after it is forwarded
across veth links into another bridge. There, since it ingresses the
bridge over veth, which doesn't have offload_fwd_mark, it triggers a
warning in nbp_switchdev_frame_mark().
Then nbp_switchdev_allowed_egress() decides not to allow egress from
this bridge through another veth, because the SKB is already marked, and
the mark (of 0) of course matches. Thus the packet is incorrectly
blocked.
Solve by resetting offload_fwd_mark() in skb_scrub_packet(). That
function is called from tunnels and also from veth, and thus catches the
cases where traffic is forwarded between bridges and transformed in a
way that invalidates the marking.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Suggested-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/skbuff.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a1be7f19d998..9a8a72cefe9b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4882,6 +4882,11 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
nf_reset(skb);
nf_reset_trace(skb);
+#ifdef CONFIG_NET_SWITCHDEV
+ skb->offload_fwd_mark = 0;
+ skb->offload_mr_fwd_mark = 0;
+#endif
+
if (!xnet)
return;
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 02/18] selftests: forwarding: lib: Support NUM_NETIFS of 0
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
So far the case of NUM_NETIFS of 0 has not been interesting. However if
one wishes to reuse the lib.sh routines in a setup of a separate
namespace, being able to import like this is handy.
Therefore replace the {1..$NUM_NETIFS} references, which cause iteration
over 1 and 0, with an explicit for loop like we do in setup_wait() and
tc_offload_check(), so that for NUM_NETIFS of 0 no iteration is done.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 85d253546684..bb0e9fdf893e 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -104,7 +104,7 @@ create_netif_veth()
{
local i
- for i in $(eval echo {1..$NUM_NETIFS}); do
+ for ((i = 1; i <= NUM_NETIFS; ++i)); do
local j=$((i+1))
ip link show dev ${NETIFS[p$i]} &> /dev/null
@@ -135,7 +135,7 @@ if [[ "$NETIF_CREATE" = "yes" ]]; then
create_netif
fi
-for i in $(eval echo {1..$NUM_NETIFS}); do
+for ((i = 1; i <= NUM_NETIFS; ++i)); do
ip link show dev ${NETIFS[p$i]} &> /dev/null
if [[ $? -ne 0 ]]; then
echo "SKIP: could not find all required interfaces"
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 09/18] selftests: forwarding: vxlan_bridge_1d: Add flood test
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Test that when sending traffic to an unlearned MAC address, the traffic
is flooded to both remote VXLAN endpoints.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/forwarding/vxlan_bridge_1d.sh | 105 ++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
index 0e3d7abc70d3..1edd5189c41c 100755
--- a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
+++ b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
@@ -66,6 +66,7 @@ export VXPORT
: ${ALL_TESTS:="
ping_ipv4
+ test_flood
"}
NUM_NETIFS=6
@@ -289,6 +290,110 @@ ping_ipv4()
ping_test $h1 192.0.2.4 ": local->remote 2"
}
+maybe_in_ns()
+{
+ echo ${1:+in_ns} $1
+}
+
+__flood_counter_add_del()
+{
+ local add_del=$1; shift
+ local dev=$1; shift
+ local ns=$1; shift
+
+ # Putting the ICMP capture both to HW and to SW will end up
+ # double-counting the packets that are trapped to slow path, such as for
+ # the unicast test. Adding either skip_hw or skip_sw fixes this problem,
+ # but with skip_hw, the flooded packets are not counted at all, because
+ # those are dropped due to MAC address mismatch; and skip_sw is a no-go
+ # for veth-based topologies.
+ #
+ # So try to install with skip_sw and fall back to skip_sw if that fails.
+
+ $(maybe_in_ns $ns) __icmp_capture_add_del \
+ $add_del 100 "" $dev skip_sw 2>/dev/null || \
+ $(maybe_in_ns $ns) __icmp_capture_add_del \
+ $add_del 100 "" $dev skip_hw
+}
+
+flood_counter_install()
+{
+ __flood_counter_add_del add "$@"
+}
+
+flood_counter_uninstall()
+{
+ __flood_counter_add_del del "$@"
+}
+
+flood_fetch_stat()
+{
+ local dev=$1; shift
+ local ns=$1; shift
+
+ $(maybe_in_ns $ns) tc_rule_stats_get $dev 100 ingress
+}
+
+flood_fetch_stats()
+{
+ local counters=("${@}")
+ local counter
+
+ for counter in "${counters[@]}"; do
+ flood_fetch_stat $counter
+ done
+}
+
+vxlan_flood_test()
+{
+ local mac=$1; shift
+ local dst=$1; shift
+ local -a expects=("${@}")
+
+ local -a counters=($h2 "vx2 ns1" "vx2 ns2")
+ local counter
+ local key
+
+ for counter in "${counters[@]}"; do
+ flood_counter_install $counter
+ done
+
+ local -a t0s=($(flood_fetch_stats "${counters[@]}"))
+ $MZ $h1 -c 10 -d 100msec -p 64 -b $mac -B $dst -t icmp -q
+ sleep 1
+ local -a t1s=($(flood_fetch_stats "${counters[@]}"))
+
+ for key in ${!t0s[@]}; do
+ local delta=$((t1s[$key] - t0s[$key]))
+ local expect=${expects[$key]}
+
+ ((expect == delta))
+ check_err $? "${counters[$key]}: Expected to capture $expect packets, got $delta."
+ done
+
+ for counter in "${counters[@]}"; do
+ flood_counter_uninstall $counter
+ done
+}
+
+__test_flood()
+{
+ local mac=$1; shift
+ local dst=$1; shift
+ local what=$1; shift
+
+ RET=0
+
+ vxlan_flood_test $mac $dst 10 10 10
+
+ log_test "VXLAN: $what"
+}
+
+test_flood()
+{
+ __test_flood de:ad:be:ef:13:37 192.0.2.100 "flood"
+}
+
test_all()
{
echo "Running tests with UDP port $VXPORT"
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 03/18] selftests: forwarding: lib: Add in_ns()
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
In order to run a certain command inside another network namespace, it's
possible to use "ip netns exec ns command". However then one can't use
functions defined in lib.sh or a test suite.
One option is to do "ip netns exec ns bash -c command", provided that
all functions that one wishes to use (and their dependencies) are
published using "export -f". That may not be practical.
Therefore, introduce a helper in_ns(), which wraps a given command in a
boilerplate of "ip netns exec" and "source lib.sh", thus making all
library functions available. (Custom functions that a script wishes to
run within a namespace still need to be exported.)
Because quotes in "$@" aren't recognized in heredoc, hand-expand the
array in an explicit for loop, leveraging printf %q to handle proper
quoting.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index bb0e9fdf893e..93d6e9df483e 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -783,6 +783,17 @@ multipath_eval()
log_info "Expected ratio $weights_ratio Measured ratio $packets_ratio"
}
+in_ns()
+{
+ local name=$1; shift
+
+ ip netns exec $name bash <<-EOF
+ NUM_NETIFS=0
+ source lib.sh
+ $(for a in "$@"; do printf "%q${IFS:0:1}" "$a"; done)
+ EOF
+}
+
##############################################################################
# Tests
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 04/18] selftests: forwarding: ping{6,}_test(): Add description argument
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Have ping_test() recognize an optional argument with a description of
the test. This is handy if there are several ping test, to make it clear
which is which.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 93d6e9df483e..fa734ddce2e9 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -813,7 +813,7 @@ ping_test()
ping_do $1 $2
check_err $?
- log_test "ping"
+ log_test "ping$3"
}
ping6_do()
@@ -832,7 +832,7 @@ ping6_test()
ping6_do $1 $2
check_err $?
- log_test "ping6"
+ log_test "ping6$3"
}
learning_test()
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 11/18] selftests: forwarding: vxlan_bridge_1d: Reconfigure & rerun tests
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
The ordering of the topology creation can have impact on whether a
driver is successful in offloading VXLAN. Therefore add a pseudo-test
that reshuffles bits of the topology, and then reruns the same suite of
tests again to make sure that the new setup is supported as well.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/forwarding/vxlan_bridge_1d.sh | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
index 1a3486ec1d21..a943d8da14b9 100755
--- a/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
+++ b/tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh
@@ -68,6 +68,10 @@ export VXPORT
ping_ipv4
test_flood
test_unicast
+ reapply_config
+ ping_ipv4
+ test_flood
+ test_unicast
"}
NUM_NETIFS=6
@@ -288,6 +292,28 @@ cleanup()
vrf_cleanup
}
+# For the first round of tests, vx1 is the first device to get attached to the
+# bridge, and that at the point that the local IP is already configured. Try the
+# other scenario of attaching the device to an already-offloaded bridge, and
+# only then attach the local IP.
+reapply_config()
+{
+ echo "Reapplying configuration"
+
+ bridge fdb del dev vx1 00:00:00:00:00:00 dst 192.0.2.50 self
+ bridge fdb del dev vx1 00:00:00:00:00:00 dst 192.0.2.34 self
+ rp1_unset_addr
+ ip link set dev vx1 nomaster
+ sleep 5
+
+ ip link set dev vx1 master br1
+ bridge fdb append dev vx1 00:00:00:00:00:00 dst 192.0.2.34 self
+ bridge fdb append dev vx1 00:00:00:00:00:00 dst 192.0.2.50 self
+ sleep 1
+ rp1_set_addr
+ sleep 5
+}
+
ping_ipv4()
{
ping_test $h1 192.0.2.2 ": local->local"
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 05/18] selftests: forwarding: ping{6,}_do(): Allow passing ping arguments
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Make the ping routine more generic by allowing passing arbitrary ping
command-line arguments.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index fa734ddce2e9..e916663a1019 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -801,10 +801,11 @@ ping_do()
{
local if_name=$1
local dip=$2
+ local args=$3
local vrf_name
vrf_name=$(master_name_get $if_name)
- ip vrf exec $vrf_name $PING $dip -c 10 -i 0.1 -w 2 &> /dev/null
+ ip vrf exec $vrf_name $PING $args $dip -c 10 -i 0.1 -w 2 &> /dev/null
}
ping_test()
@@ -820,10 +821,11 @@ ping6_do()
{
local if_name=$1
local dip=$2
+ local args=$3
local vrf_name
vrf_name=$(master_name_get $if_name)
- ip vrf exec $vrf_name $PING6 $dip -c 10 -i 0.1 -w 2 &> /dev/null
+ ip vrf exec $vrf_name $PING6 $args $dip -c 10 -i 0.1 -w 2 &> /dev/null
}
ping6_test()
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 06/18] selftests: forwarding: lib: Add link_stats_rx_errors_get()
From: Ido Schimmel @ 2018-11-19 16:11 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Cc: davem@davemloft.net, shuah@kernel.org, Jiri Pirko, Petr Machata,
roopa@cumulusnetworks.com, mlxsw, Ido Schimmel
In-Reply-To: <20181119161006.5405-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Such a function will be useful for counting malformed packets in the ECN
decap test.
To that end, introduce a common handler for handling stat-fetching, and
reuse it in link_stats_tx_packets_get() and link_stats_rx_errors_get().
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index e916663a1019..7af5a03bcb32 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -477,11 +477,24 @@ master_name_get()
ip -j link show dev $if_name | jq -r '.[]["master"]'
}
+link_stats_get()
+{
+ local if_name=$1; shift
+ local dir=$1; shift
+ local stat=$1; shift
+
+ ip -j -s link show dev $if_name \
+ | jq '.[]["stats64"]["'$dir'"]["'$stat'"]'
+}
+
link_stats_tx_packets_get()
{
- local if_name=$1
+ link_stats_get $1 tx packets
+}
- ip -j -s link show dev $if_name | jq '.[]["stats64"]["tx"]["packets"]'
+link_stats_rx_errors_get()
+{
+ link_stats_get $1 rx errors
}
tc_rule_stats_get()
--
2.19.1
^ permalink raw reply related
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