* [PATCH net-next 2/3 v2] macsec: remove superfluous zeroing of skb reference
From: Andreas Steinmetz @ 2019-07-02 20:50 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca
Remove superfluous zeroing of skb pointer for the RX_HANDLER_CONSUMED
case, since in that case, __netif_receive_skb_core will simply ignore
the value.
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-06-30 22:04:10.003613880 +0200
+++ b/drivers/net/macsec.c 2019-06-30 22:04:22.405822436 +0200
@@ -1215,7 +1215,6 @@
macsec_rxsc_put(rx_sc);
}
rcu_read_unlock();
- *pskb = NULL;
return RX_HANDLER_CONSUMED;
}
@@ -1239,7 +1238,6 @@
rcu_read_unlock();
- *pskb = NULL;
return RX_HANDLER_CONSUMED;
drop:
@@ -1249,7 +1247,6 @@
rcu_read_unlock();
drop_direct:
kfree_skb(skb);
- *pskb = NULL;
return RX_HANDLER_CONSUMED;
nosci:
^ permalink raw reply
* [PATCH net-next 1/3 v2] macsec: remove superfluous function calls
From: Andreas Steinmetz @ 2019-07-02 20:49 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca
Remove superfluous skb_share_check() and skb_unshare().
macsec_decrypt is only called by macsec_handle_frame which
already does a skb_unshare().
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-06-30 22:02:54.906908179 +0200
+++ b/drivers/net/macsec.c 2019-06-30 22:03:07.315785186 +0200
@@ -939,9 +939,6 @@
u16 icv_len = secy->icv_len;
macsec_skb_cb(skb)->valid = false;
- skb = skb_share_check(skb, GFP_ATOMIC);
- if (!skb)
- return ERR_PTR(-ENOMEM);
ret = skb_cow_data(skb, 0, &trailer);
if (unlikely(ret < 0)) {
@@ -973,11 +970,6 @@
aead_request_set_crypt(req, sg, sg, len, iv);
aead_request_set_ad(req, macsec_hdr_len(macsec_skb_cb(skb)->has_sci));
- skb = skb_unshare(skb, GFP_ATOMIC);
- if (!skb) {
- aead_request_free(req);
- return ERR_PTR(-ENOMEM);
- }
} else {
/* integrity only: all headers + data authenticated */
aead_request_set_crypt(req, sg, sg, icv_len, iv);
^ permalink raw reply
* [PATCH] gve: Fix u64_stats_sync to initialize start
From: Catherine Sullivan @ 2019-07-02 22:46 UTC (permalink / raw)
To: netdev; +Cc: Catherine Sullivan, kbuild test robot
u64_stats_fetch_begin needs to initialize start.
Signed-off-by: Catherine Sullivan <csully@google.com>
Reported-by: kbuild test robot <lkp@intel.com>
---
drivers/net/ethernet/google/gve/gve_ethtool.c | 6 ++++--
drivers/net/ethernet/google/gve/gve_main.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index 52947d668e6d..26540b856541 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -102,7 +102,8 @@ gve_get_ethtool_stats(struct net_device *netdev,
ring < priv->rx_cfg.num_queues; ring++) {
if (priv->rx) {
do {
- u64_stats_fetch_begin(&priv->rx[ring].statss);
+ start =
+ u64_stats_fetch_begin(&priv->rx[ring].statss);
rx_pkts += priv->rx[ring].rpackets;
rx_bytes += priv->rx[ring].rbytes;
} while (u64_stats_fetch_retry(&priv->rx[ring].statss,
@@ -113,7 +114,8 @@ gve_get_ethtool_stats(struct net_device *netdev,
ring < priv->tx_cfg.num_queues; ring++) {
if (priv->tx) {
do {
- u64_stats_fetch_begin(&priv->tx[ring].statss);
+ start =
+ u64_stats_fetch_begin(&priv->tx[ring].statss);
tx_pkts += priv->tx[ring].pkt_done;
tx_bytes += priv->tx[ring].bytes_done;
} while (u64_stats_fetch_retry(&priv->tx[ring].statss,
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 6a147ed4627f..83f65a5a9a3f 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -35,7 +35,8 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
if (priv->rx) {
for (ring = 0; ring < priv->rx_cfg.num_queues; ring++) {
do {
- u64_stats_fetch_begin(&priv->rx[ring].statss);
+ start =
+ u64_stats_fetch_begin(&priv->rx[ring].statss);
s->rx_packets += priv->rx[ring].rpackets;
s->rx_bytes += priv->rx[ring].rbytes;
} while (u64_stats_fetch_retry(&priv->rx[ring].statss,
@@ -45,7 +46,8 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
if (priv->tx) {
for (ring = 0; ring < priv->tx_cfg.num_queues; ring++) {
do {
- u64_stats_fetch_begin(&priv->tx[ring].statss);
+ start =
+ u64_stats_fetch_begin(&priv->tx[ring].statss);
s->tx_packets += priv->tx[ring].pkt_done;
s->tx_bytes += priv->tx[ring].bytes_done;
} while (u64_stats_fetch_retry(&priv->rx[ring].statss,
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net-next v5 5/5] selftests: tc-tests: actions: add MPLS tests
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
In-Reply-To: <1562113531-29296-1-git-send-email-john.hurley@netronome.com>
Add a new series of selftests to verify the functionality of act_mpls in
TC.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../tc-testing/tc-tests/actions/mpls.json | 812 +++++++++++++++++++++
1 file changed, 812 insertions(+)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json b/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
new file mode 100644
index 0000000..d46180a
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
@@ -0,0 +1,812 @@
+[
+ {
+ "id": "a933",
+ "name": "Add MPLS dec_ttl action with pipe opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl pipe index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*pipe.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "08d1",
+ "name": "Add mpls dec_ttl action with pass opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl pass index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action mpls index 8",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*pass.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "d786",
+ "name": "Add mpls dec_ttl action with drop opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl drop index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action mpls index 8",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*drop.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "f334",
+ "name": "Add mpls dec_ttl action with reclassify opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl reclassify index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action mpls index 8",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*reclassify.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "29bd",
+ "name": "Add mpls dec_ttl action with continue opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl continue index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action mpls index 8",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*continue.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "48df",
+ "name": "Add mpls dec_ttl action with jump opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl jump 10 index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*jump 10.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "62eb",
+ "name": "Add mpls dec_ttl action with trap opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl trap index 8",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl trap.*index 8 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "9118",
+ "name": "Add mpls dec_ttl action with invalid opcode",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl foo index 8",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*foo.*index 8 ref",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "6ce1",
+ "name": "Add mpls dec_ttl action with label (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl label 20",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*label.*20.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "352f",
+ "name": "Add mpls dec_ttl action with tc (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl tc 3",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*tc.*3.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "fa1c",
+ "name": "Add mpls dec_ttl action with ttl (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl ttl 20",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*ttl.*20.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "6b79",
+ "name": "Add mpls dec_ttl action with bos (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls dec_ttl bos 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*bos.*1.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "d4c4",
+ "name": "Add mpls pop action with ip proto",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*protocol.*ip.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "92fe",
+ "name": "Add mpls pop action with mpls proto",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol mpls_mc",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*protocol.*mpls_mc.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "7e23",
+ "name": "Add mpls pop action with no protocol (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "6182",
+ "name": "Add mpls pop action with label (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 label 20",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*label.*20.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "6475",
+ "name": "Add mpls pop action with tc (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 tc 3",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*tc.*3.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "067b",
+ "name": "Add mpls pop action with ttl (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 ttl 20",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*ttl.*20.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "7316",
+ "name": "Add mpls pop action with bos (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 bos 1",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*bos.*1.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "38cc",
+ "name": "Add mpls push action with label",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 20",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*[0-9]+.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "c281",
+ "name": "Add mpls push action with mpls_mc protocol",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push protocol mpls_mc label 20",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_mc.*label.*20.*ttl.*[0-9]+.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "5db4",
+ "name": "Add mpls push action with label, tc and ttl",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 20 tc 3 ttl 128",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*tc.*3.*ttl.*128.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "16eb",
+ "name": "Add mpls push action with label and bos",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 20 bos 1",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*bos.*1.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "d69d",
+ "name": "Add mpls push action with no label (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "e8e4",
+ "name": "Add mpls push action with ipv4 protocol (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push protocol ipv4 label 20",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*[0-9]+.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "ecd0",
+ "name": "Add mpls push action with out of range label (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 1048576",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*1048576.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "d303",
+ "name": "Add mpls push action with out of range tc (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 20 tc 8",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*tc.*8.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "fd6e",
+ "name": "Add mpls push action with ttl of 0 (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls push label 20 ttl 0",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*0.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "19e9",
+ "name": "Add mpls mod action with mpls label",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls mod label 20",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*modify.*label.*20.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "04b5",
+ "name": "Add mpls mod action with mpls tc",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls mod tc 3",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*modify.*tc.*3.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "6ed5",
+ "name": "Add mpls mod action with mpls label",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls mod ttl 128",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*modify.*ttl.*128.*pipe",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "db7c",
+ "name": "Add mpls mod action with protocol (invalid)",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action mpls mod protocol ipv4",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*modify.*protocol.*ip.*pipe",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "b070",
+ "name": "Replace existing mpls push action with new ID",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action mpls push label 20 pipe index 12"
+ ],
+ "cmdUnderTest": "$TC actions replace action mpls push label 30 pipe index 12",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions get action mpls index 12",
+ "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*30.*pipe.*index 12 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action mpls"
+ ]
+ },
+ {
+ "id": "6cce",
+ "name": "Delete mpls pop action",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action mpls pop protocol ipv4 index 44"
+ ],
+ "cmdUnderTest": "$TC actions del action mpls index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*pop.*index 44 ref",
+ "matchCount": "0",
+ "teardown": []
+ },
+ {
+ "id": "d138",
+ "name": "Flush mpls actions",
+ "category": [
+ "actions",
+ "mpls"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action mpls",
+ 0,
+ 1,
+ 255
+ ],
+ "$TC actions add action mpls push label 10 index 10",
+ "$TC actions add action mpls push label 20 index 20",
+ "$TC actions add action mpls push label 30 index 30",
+ "$TC actions add action mpls push label 40 index 40"
+ ],
+ "cmdUnderTest": "$TC actions flush action mpls",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action mpls",
+ "matchPattern": "action order [0-9]+: mpls.*push.*",
+ "matchCount": "0",
+ "teardown": []
+ }
+]
--
2.7.4
^ permalink raw reply related
* [PATCH v3 bpf-next] bpf: Add support for fq's EDT to HBM
From: brakmo @ 2019-07-02 22:01 UTC (permalink / raw)
To: netdev
Cc: Alexei Starovoitov, Yonghong Song, Daniel Borkmann, Eric Dumazet,
Kernel Team
Adds support for fq's Earliest Departure Time to HBM (Host Bandwidth
Manager). Includes a new BPF program supporting EDT, and also updates
corresponding programs.
It will drop packets with an EDT of more than 500us in the future
unless the packet belongs to a flow with less than 2 packets in flight.
This is done so each flow has at least 2 packets in flight, so they
will not starve, and also to help prevent delayed ACK timeouts.
It will also work with ECN enabled traffic, where the packets will be
CE marked if their EDT is more than 50us in the future.
The table below shows some performance numbers. The flows are back to
back RPCS. One server sending to another, either 2 or 4 flows.
One flow is a 10KB RPC, the rest are 1MB RPCs. When there are more
than one flow of a given RPC size, the numbers represent averages.
The rate limit applies to all flows (they are in the same cgroup).
Tests ending with "-edt" ran with the new BPF program supporting EDT.
Tests ending with "-hbt" ran on top HBT qdisc with the specified rate
(i.e. no HBM). The other tests ran with the HBM BPF program included
in the HBM patch-set.
EDT has limited value when using DCTCP, but it helps in many cases when
using Cubic. It usually achieves larger link utilization and lower
99% latencies for the 1MB RPCs.
HBM ends up queueing a lot of packets with its default parameter values,
reducing the goodput of the 10KB RPCs and increasing their latency. Also,
the RTTs seen by the flows are quite large.
Aggr 10K 10K 10K 1MB 1MB 1MB
Limit rate drops RTT rate P90 P99 rate P90 P99
Test rate Flows Mbps % us Mbps us us Mbps ms ms
-------- ---- ----- ---- ----- --- ---- ---- ---- ---- ---- ----
cubic 1G 2 904 0.02 108 257 511 539 647 13.4 24.5
cubic-edt 1G 2 982 0.01 156 239 656 967 743 14.0 17.2
dctcp 1G 2 977 0.00 105 324 408 744 653 14.5 15.9
dctcp-edt 1G 2 981 0.01 142 321 417 811 660 15.7 17.0
cubic-htb 1G 2 919 0.00 1825 40 2822 4140 879 9.7 9.9
cubic 200M 2 155 0.30 220 81 532 655 74 283 450
cubic-edt 200M 2 188 0.02 222 87 1035 1095 101 84 85
dctcp 200M 2 188 0.03 111 77 912 939 111 76 325
dctcp-edt 200M 2 188 0.03 217 74 1416 1738 114 76 79
cubic-htb 200M 2 188 0.00 5015 8 14ms 15ms 180 48 50
cubic 1G 4 952 0.03 110 165 516 546 262 38 154
cubic-edt 1G 4 973 0.01 190 111 1034 1314 287 65 79
dctcp 1G 4 951 0.00 103 180 617 905 257 37 38
dctcp-edt 1G 4 967 0.00 163 151 732 1126 272 43 55
cubic-htb 1G 4 914 0.00 3249 13 7ms 8ms 300 29 34
cubic 5G 4 4236 0.00 134 305 490 624 1310 10 17
cubic-edt 5G 4 4865 0.00 156 306 425 759 1520 10 16
dctcp 5G 4 4936 0.00 128 485 221 409 1484 7 9
dctcp-edt 5G 4 4924 0.00 148 390 392 623 1508 11 26
v1 -> v2: Incorporated Andrii's suggestions
v2 -> v3: Incorporated Yonghong's suggestions
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
samples/bpf/Makefile | 2 +
samples/bpf/do_hbm_test.sh | 22 ++---
samples/bpf/hbm.c | 18 +++-
samples/bpf/hbm_edt_kern.c | 169 +++++++++++++++++++++++++++++++++++++
samples/bpf/hbm_kern.h | 40 +++++++--
5 files changed, 232 insertions(+), 19 deletions(-)
create mode 100644 samples/bpf/hbm_edt_kern.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 0917f8cf4fab..35640414ebb3 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -168,6 +168,7 @@ always += task_fd_query_kern.o
always += xdp_sample_pkts_kern.o
always += ibumad_kern.o
always += hbm_out_kern.o
+always += hbm_edt_kern.o
KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/
@@ -272,6 +273,7 @@ $(src)/*.c: verify_target_bpf $(LIBBPF)
$(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
$(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
$(obj)/hbm.o: $(src)/hbm.h
+$(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
# But, there is no easy way to fix it, so just exclude it since it is
diff --git a/samples/bpf/do_hbm_test.sh b/samples/bpf/do_hbm_test.sh
index e48b047d4646..ffe4c0607341 100755
--- a/samples/bpf/do_hbm_test.sh
+++ b/samples/bpf/do_hbm_test.sh
@@ -14,7 +14,7 @@ Usage() {
echo "loads. The output is the goodput in Mbps (unless -D was used)."
echo ""
echo "USAGE: $name [out] [-b=<prog>|--bpf=<prog>] [-c=<cc>|--cc=<cc>]"
- echo " [-D] [-d=<delay>|--delay=<delay>] [--debug] [-E]"
+ echo " [-D] [-d=<delay>|--delay=<delay>] [--debug] [-E] [--edt]"
echo " [-f=<#flows>|--flows=<#flows>] [-h] [-i=<id>|--id=<id >]"
echo " [-l] [-N] [--no_cn] [-p=<port>|--port=<port>] [-P]"
echo " [-q=<qdisc>] [-R] [-s=<server>|--server=<server]"
@@ -30,6 +30,7 @@ Usage() {
echo " other detailed information. This information is"
echo " test dependent (i.e. iperf3 or netperf)."
echo " -E enable ECN (not required for dctcp)"
+ echo " --edt use fq's Earliest Departure Time (requires fq)"
echo " -f or --flows number of concurrent flows (default=1)"
echo " -i or --id cgroup id (an integer, default is 1)"
echo " -N use netperf instead of iperf3"
@@ -130,13 +131,12 @@ processArgs () {
details=1
;;
-E)
- ecn=1
+ ecn=1
+ ;;
+ --edt)
+ flags="$flags --edt"
+ qdisc="fq"
;;
- # Support for upcomming fq Early Departure Time egress rate limiting
- #--edt)
- # prog="hbm_out_edt_kern.o"
- # qdisc="fq"
- # ;;
-f=*|--flows=*)
flows="${i#*=}"
;;
@@ -228,8 +228,8 @@ if [ "$netem" -ne "0" ] ; then
tc qdisc del dev lo root > /dev/null 2>&1
tc qdisc add dev lo root netem delay $netem\ms > /dev/null 2>&1
elif [ "$qdisc" != "" ] ; then
- tc qdisc del dev lo root > /dev/null 2>&1
- tc qdisc add dev lo root $qdisc > /dev/null 2>&1
+ tc qdisc del dev eth0 root > /dev/null 2>&1
+ tc qdisc add dev eth0 root $qdisc > /dev/null 2>&1
fi
n=0
@@ -399,7 +399,9 @@ fi
if [ "$netem" -ne "0" ] ; then
tc qdisc del dev lo root > /dev/null 2>&1
fi
-
+if [ "$qdisc" != "" ] ; then
+ tc qdisc del dev eth0 root > /dev/null 2>&1
+fi
sleep 2
hbmPid=`ps ax | grep "hbm " | grep --invert-match "grep" | awk '{ print $1 }'`
diff --git a/samples/bpf/hbm.c b/samples/bpf/hbm.c
index b905b32ff185..e0fbab9bec83 100644
--- a/samples/bpf/hbm.c
+++ b/samples/bpf/hbm.c
@@ -62,6 +62,7 @@ bool loopback_flag;
bool debugFlag;
bool work_conserving_flag;
bool no_cn_flag;
+bool edt_flag;
static void Usage(void);
static void read_trace_pipe2(void);
@@ -372,9 +373,14 @@ static int run_bpf_prog(char *prog, int cg_id)
fprintf(fout, "avg rtt:%d\n",
(int)(qstats.sum_rtt / (qstats.pkts_total + 1)));
// Average credit
- fprintf(fout, "avg credit:%d\n",
- (int)(qstats.sum_credit /
- (1500 * ((int)qstats.pkts_total) + 1)));
+ if (edt_flag)
+ fprintf(fout, "avg credit_ms:%.03f\n",
+ (qstats.sum_credit /
+ (qstats.pkts_total + 1.0)) / 1000000.0);
+ else
+ fprintf(fout, "avg credit:%d\n",
+ (int)(qstats.sum_credit /
+ (1500 * ((int)qstats.pkts_total ) + 1)));
// Return values stats
for (k = 0; k < RET_VAL_COUNT; k++) {
@@ -408,6 +414,7 @@ static void Usage(void)
" Where:\n"
" -o indicates egress direction (default)\n"
" -d print BPF trace debug buffer\n"
+ " --edt use fq's Earliest Departure Time\n"
" -l also limit flows using loopback\n"
" -n <#> to create cgroup \"/hbm#\" and attach prog\n"
" Default is /hbm1\n"
@@ -433,6 +440,7 @@ int main(int argc, char **argv)
char *optstring = "iodln:r:st:wh";
struct option loptions[] = {
{"no_cn", 0, NULL, 1},
+ {"edt", 0, NULL, 2},
{NULL, 0, NULL, 0}
};
@@ -441,6 +449,10 @@ int main(int argc, char **argv)
case 1:
no_cn_flag = true;
break;
+ case 2:
+ prog = "hbm_edt_kern.o";
+ edt_flag = true;
+ break;
case'o':
break;
case 'd':
diff --git a/samples/bpf/hbm_edt_kern.c b/samples/bpf/hbm_edt_kern.c
new file mode 100644
index 000000000000..df477dd02f59
--- /dev/null
+++ b/samples/bpf/hbm_edt_kern.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * Sample Host Bandwidth Manager (HBM) BPF program.
+ *
+ * A cgroup skb BPF egress program to limit cgroup output bandwidth.
+ * It uses a modified virtual token bucket queue to limit average
+ * egress bandwidth. The implementation uses credits instead of tokens.
+ * Negative credits imply that queueing would have happened (this is
+ * a virtual queue, so no queueing is done by it. However, queueing may
+ * occur at the actual qdisc (which is not used for rate limiting).
+ *
+ * This implementation uses 3 thresholds, one to start marking packets and
+ * the other two to drop packets:
+ * CREDIT
+ * - <--------------------------|------------------------> +
+ * | | | 0
+ * | Large pkt |
+ * | drop thresh |
+ * Small pkt drop Mark threshold
+ * thresh
+ *
+ * The effect of marking depends on the type of packet:
+ * a) If the packet is ECN enabled and it is a TCP packet, then the packet
+ * is ECN marked.
+ * b) If the packet is a TCP packet, then we probabilistically call tcp_cwr
+ * to reduce the congestion window. The current implementation uses a linear
+ * distribution (0% probability at marking threshold, 100% probability
+ * at drop threshold).
+ * c) If the packet is not a TCP packet, then it is dropped.
+ *
+ * If the credit is below the drop threshold, the packet is dropped. If it
+ * is a TCP packet, then it also calls tcp_cwr since packets dropped by
+ * by a cgroup skb BPF program do not automatically trigger a call to
+ * tcp_cwr in the current kernel code.
+ *
+ * This BPF program actually uses 2 drop thresholds, one threshold
+ * for larger packets (>= 120 bytes) and another for smaller packets. This
+ * protects smaller packets such as SYNs, ACKs, etc.
+ *
+ * The default bandwidth limit is set at 1Gbps but this can be changed by
+ * a user program through a shared BPF map. In addition, by default this BPF
+ * program does not limit connections using loopback. This behavior can be
+ * overwritten by the user program. There is also an option to calculate
+ * some statistics, such as percent of packets marked or dropped, which
+ * a user program, such as hbm, can access.
+ */
+
+#include "hbm_kern.h"
+
+SEC("cgroup_skb/egress")
+int _hbm_out_cg(struct __sk_buff *skb)
+{
+ long long delta = 0, delta_send;
+ unsigned long long curtime, sendtime;
+ struct hbm_queue_stats *qsp = NULL;
+ unsigned int queue_index = 0;
+ bool congestion_flag = false;
+ bool ecn_ce_flag = false;
+ struct hbm_pkt_info pkti;
+ struct hbm_vqueue *qdp;
+ bool drop_flag = false;
+ bool cwr_flag = false;
+ int len = skb->len;
+ int rv = ALLOW_PKT;
+
+ qsp = bpf_map_lookup_elem(&queue_stats, &queue_index);
+
+ // Check if we should ignore loopback traffic
+ if (qsp != NULL && !qsp->loopback && (skb->ifindex == 1))
+ return ALLOW_PKT;
+
+ hbm_get_pkt_info(skb, &pkti);
+
+ // We may want to account for the length of headers in len
+ // calculation, like ETH header + overhead, specially if it
+ // is a gso packet. But I am not doing it right now.
+
+ qdp = bpf_get_local_storage(&queue_state, 0);
+ if (!qdp)
+ return ALLOW_PKT;
+ if (qdp->lasttime == 0)
+ hbm_init_edt_vqueue(qdp, 1024);
+
+ curtime = bpf_ktime_get_ns();
+
+ // Begin critical section
+ bpf_spin_lock(&qdp->lock);
+ delta = qdp->lasttime - curtime;
+ // bound bursts to 100us
+ if (delta < -BURST_SIZE_NS) {
+ // negative delta is a credit that allows bursts
+ qdp->lasttime = curtime - BURST_SIZE_NS;
+ delta = -BURST_SIZE_NS;
+ }
+ sendtime = qdp->lasttime;
+ delta_send = BYTES_TO_NS(len, qdp->rate);
+ __sync_add_and_fetch(&(qdp->lasttime), delta_send);
+ bpf_spin_unlock(&qdp->lock);
+ // End critical section
+
+ // Set EDT of packet
+ skb->tstamp = sendtime;
+
+ // Check if we should update rate
+ if (qsp != NULL && (qsp->rate * 128) != qdp->rate)
+ qdp->rate = qsp->rate * 128;
+
+ // Set flags (drop, congestion, cwr)
+ // last packet will be sent in the future, bound latency
+ if (delta > DROP_THRESH_NS || (delta > LARGE_PKT_DROP_THRESH_NS &&
+ len > LARGE_PKT_THRESH)) {
+ drop_flag = true;
+ if (pkti.is_tcp && pkti.ecn == 0)
+ cwr_flag = true;
+ } else if (delta > MARK_THRESH_NS) {
+ if (pkti.is_tcp)
+ congestion_flag = true;
+ else
+ drop_flag = true;
+ }
+
+ if (congestion_flag) {
+ if (bpf_skb_ecn_set_ce(skb)) {
+ ecn_ce_flag = true;
+ } else {
+ if (pkti.is_tcp) {
+ unsigned int rand = bpf_get_prandom_u32();
+
+ if (delta >= MARK_THRESH_NS +
+ (rand % MARK_REGION_SIZE_NS)) {
+ // Do congestion control
+ cwr_flag = true;
+ }
+ } else if (len > LARGE_PKT_THRESH) {
+ // Problem if too many small packets?
+ drop_flag = true;
+ congestion_flag = false;
+ }
+ }
+ }
+
+ if (pkti.is_tcp && drop_flag && pkti.packets_out <= 1) {
+ drop_flag = false;
+ cwr_flag = true;
+ congestion_flag = false;
+ }
+
+ if (qsp != NULL && qsp->no_cn)
+ cwr_flag = false;
+
+ hbm_update_stats(qsp, len, curtime, congestion_flag, drop_flag,
+ cwr_flag, ecn_ce_flag, &pkti, (int) delta);
+
+ if (drop_flag) {
+ __sync_add_and_fetch(&(qdp->credit), len);
+ __sync_add_and_fetch(&(qdp->lasttime), -delta_send);
+ rv = DROP_PKT;
+ }
+
+ if (cwr_flag)
+ rv |= CWR;
+ return rv;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/hbm_kern.h b/samples/bpf/hbm_kern.h
index be19cf1d5cd5..aa207a2eebbd 100644
--- a/samples/bpf/hbm_kern.h
+++ b/samples/bpf/hbm_kern.h
@@ -29,6 +29,7 @@
#define DROP_PKT 0
#define ALLOW_PKT 1
#define TCP_ECN_OK 1
+#define CWR 2
#ifndef HBM_DEBUG // Define HBM_DEBUG to enable debugging
#undef bpf_printk
@@ -45,8 +46,18 @@
#define MAX_CREDIT (100 * MAX_BYTES_PER_PACKET)
#define INIT_CREDIT (INITIAL_CREDIT_PACKETS * MAX_BYTES_PER_PACKET)
+// Time base accounting for fq's EDT
+#define BURST_SIZE_NS 100000 // 100us
+#define MARK_THRESH_NS 50000 // 50us
+#define DROP_THRESH_NS 500000 // 500us
+// Reserve 20us of queuing for small packets (less than 120 bytes)
+#define LARGE_PKT_DROP_THRESH_NS (DROP_THRESH_NS - 20000)
+#define MARK_REGION_SIZE_NS (LARGE_PKT_DROP_THRESH_NS - MARK_THRESH_NS)
+
// rate in bytes per ns << 20
#define CREDIT_PER_NS(delta, rate) ((((u64)(delta)) * (rate)) >> 20)
+#define BYTES_PER_NS(delta, rate) ((((u64)(delta)) * (rate)) >> 20)
+#define BYTES_TO_NS(bytes, rate) div64_u64(((u64)(bytes)) << 20, (u64)(rate))
struct bpf_map_def SEC("maps") queue_state = {
.type = BPF_MAP_TYPE_CGROUP_STORAGE,
@@ -67,6 +78,7 @@ BPF_ANNOTATE_KV_PAIR(queue_stats, int, struct hbm_queue_stats);
struct hbm_pkt_info {
int cwnd;
int rtt;
+ int packets_out;
bool is_ip;
bool is_tcp;
short ecn;
@@ -86,16 +98,20 @@ static int get_tcp_info(struct __sk_buff *skb, struct hbm_pkt_info *pkti)
if (tp) {
pkti->cwnd = tp->snd_cwnd;
pkti->rtt = tp->srtt_us >> 3;
+ pkti->packets_out = tp->packets_out;
return 0;
}
}
}
}
+ pkti->cwnd = 0;
+ pkti->rtt = 0;
+ pkti->packets_out = 0;
return 1;
}
-static __always_inline void hbm_get_pkt_info(struct __sk_buff *skb,
- struct hbm_pkt_info *pkti)
+static void hbm_get_pkt_info(struct __sk_buff *skb,
+ struct hbm_pkt_info *pkti)
{
struct iphdr iph;
struct ipv6hdr *ip6h;
@@ -123,10 +139,22 @@ static __always_inline void hbm_get_pkt_info(struct __sk_buff *skb,
static __always_inline void hbm_init_vqueue(struct hbm_vqueue *qdp, int rate)
{
- bpf_printk("Initializing queue_state, rate:%d\n", rate * 128);
- qdp->lasttime = bpf_ktime_get_ns();
- qdp->credit = INIT_CREDIT;
- qdp->rate = rate * 128;
+ bpf_printk("Initializing queue_state, rate:%d\n", rate * 128);
+ qdp->lasttime = bpf_ktime_get_ns();
+ qdp->credit = INIT_CREDIT;
+ qdp->rate = rate * 128;
+}
+
+static __always_inline void hbm_init_edt_vqueue(struct hbm_vqueue *qdp,
+ int rate)
+{
+ unsigned long long curtime;
+
+ curtime = bpf_ktime_get_ns();
+ bpf_printk("Initializing queue_state, rate:%d\n", rate * 128);
+ qdp->lasttime = curtime - BURST_SIZE_NS; // support initial burst
+ qdp->credit = 0; // not used
+ qdp->rate = rate * 128;
}
static __always_inline void hbm_update_stats(struct hbm_queue_stats *qsp,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v5 0/5] Add MPLS actions to TC
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
This patchset introduces a new TC action module that allows the
manipulation of the MPLS headers of packets. The code impliments
functionality including push, pop, and modify.
Also included are tests for the new funtionality. Note that these will
require iproute2 changes to be submitted soon.
NOTE: these patches are applied to net-next along with the patch:
[PATCH net 1/1] net: openvswitch: fix csum updates for MPLS actions
This patch has been accepted into net but, at time of posting, is not yet
in net-next.
v4-v5:
- move mpls_hdr() call to after skb_ensure_writable - patch 3
(Willem de Bruijn)
- move mpls_dec_ttl to helper - patch 4 (Willem de Bruijn)
- add iproute2 usage example to commit msg - patch 4 (David Ahern)
- align label validation with mpls core code - patch 4 (David Ahern)
- improve extack message for no proto in mpls pop - patch 4 (David Ahern)
v3-v4:
- refactor and reuse OvS code (Cong Wang)
- use csum API rather than skb_post*rscum to update skb->csum (Cong Wang)
- remove unnecessary warning (Cong Wang)
- add comments to uapi attributes (David Ahern)
- set strict type policy check for TCA_MPLS_UNSPEC (David Ahern)
- expand/improve extack messages (David Ahern)
- add option to manually set BOS
v2-v3:
- remove a few unnecessary line breaks (Jiri Pirko)
- retract hw offload patch from set (resubmit with driver changes) (Jiri)
v1->v2:
- ensure TCA_ID_MPLS does not conflict with TCA_ID_CTINFO (Davide Caratti)
John Hurley (5):
net: core: move push MPLS functionality from OvS to core helper
net: core: move pop MPLS functionality from OvS to core helper
net: core: add MPLS update core helper and use in OvS
net: sched: add mpls manipulation actions to TC
selftests: tc-tests: actions: add MPLS tests
include/linux/skbuff.h | 4 +
include/net/tc_act/tc_mpls.h | 30 +
include/uapi/linux/pkt_cls.h | 3 +-
include/uapi/linux/tc_act/tc_mpls.h | 33 +
net/core/skbuff.c | 169 +++++
net/openvswitch/actions.c | 81 +-
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/act_mpls.c | 406 +++++++++++
.../tc-testing/tc-tests/actions/mpls.json | 812 +++++++++++++++++++++
10 files changed, 1477 insertions(+), 73 deletions(-)
create mode 100644 include/net/tc_act/tc_mpls.h
create mode 100644 include/uapi/linux/tc_act/tc_mpls.h
create mode 100644 net/sched/act_mpls.c
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
--
2.7.4
^ permalink raw reply
* [PATCH net-next v5 4/5] net: sched: add mpls manipulation actions to TC
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
In-Reply-To: <1562113531-29296-1-git-send-email-john.hurley@netronome.com>
Currently, TC offers the ability to match on the MPLS fields of a packet
through the use of the flow_dissector_key_mpls struct. However, as yet, TC
actions do not allow the modification or manipulation of such fields.
Add a new module that registers TC action ops to allow manipulation of
MPLS. This includes the ability to push and pop headers as well as modify
the contents of new or existing headers. A further action to decrement the
TTL field of an MPLS header is also provided with a new helper added to
support this.
Examples of the usage of the new action with flower rules to push and pop
MPLS labels are:
tc filter add dev eth0 protocol ip parent ffff: flower \
action mpls push protocol mpls_uc label 123 \
action mirred egress redirect dev eth1
tc filter add dev eth0 protocol mpls_uc parent ffff: flower \
action mpls pop protocol ipv4 \
action mirred egress redirect dev eth1
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
include/linux/skbuff.h | 1 +
include/net/tc_act/tc_mpls.h | 30 +++
include/uapi/linux/pkt_cls.h | 3 +-
include/uapi/linux/tc_act/tc_mpls.h | 33 +++
net/core/skbuff.c | 30 +++
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/act_mpls.c | 406 ++++++++++++++++++++++++++++++++++++
8 files changed, 514 insertions(+), 1 deletion(-)
create mode 100644 include/net/tc_act/tc_mpls.h
create mode 100644 include/uapi/linux/tc_act/tc_mpls.h
create mode 100644 net/sched/act_mpls.c
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1545c4c..a9f8ff9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3449,6 +3449,7 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto);
int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto);
int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse);
+int skb_mpls_dec_ttl(struct sk_buff *skb);
struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy,
gfp_t gfp);
diff --git a/include/net/tc_act/tc_mpls.h b/include/net/tc_act/tc_mpls.h
new file mode 100644
index 0000000..4bc3d92
--- /dev/null
+++ b/include/net/tc_act/tc_mpls.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#ifndef __NET_TC_MPLS_H
+#define __NET_TC_MPLS_H
+
+#include <linux/tc_act/tc_mpls.h>
+#include <net/act_api.h>
+
+struct tcf_mpls_params {
+ int tcfm_action;
+ u32 tcfm_label;
+ u8 tcfm_tc;
+ u8 tcfm_ttl;
+ u8 tcfm_bos;
+ __be16 tcfm_proto;
+ struct rcu_head rcu;
+};
+
+#define ACT_MPLS_TC_NOT_SET 0xff
+#define ACT_MPLS_BOS_NOT_SET 0xff
+#define ACT_MPLS_LABEL_NOT_SET 0xffffffff
+
+struct tcf_mpls {
+ struct tc_action common;
+ struct tcf_mpls_params __rcu *mpls_p;
+};
+#define to_mpls(a) ((struct tcf_mpls *)a)
+
+#endif /* __NET_TC_MPLS_H */
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 8cc6b67..e22ef4a 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -104,8 +104,9 @@ enum tca_id {
TCA_ID_SIMP = TCA_ACT_SIMP,
TCA_ID_IFE = TCA_ACT_IFE,
TCA_ID_SAMPLE = TCA_ACT_SAMPLE,
- /* other actions go here */
TCA_ID_CTINFO,
+ TCA_ID_MPLS,
+ /* other actions go here */
__TCA_ID_MAX = 255
};
diff --git a/include/uapi/linux/tc_act/tc_mpls.h b/include/uapi/linux/tc_act/tc_mpls.h
new file mode 100644
index 0000000..9360e95
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_mpls.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#ifndef __LINUX_TC_MPLS_H
+#define __LINUX_TC_MPLS_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_MPLS_ACT_POP 1
+#define TCA_MPLS_ACT_PUSH 2
+#define TCA_MPLS_ACT_MODIFY 3
+#define TCA_MPLS_ACT_DEC_TTL 4
+
+struct tc_mpls {
+ tc_gen; /* generic TC action fields. */
+ int m_action; /* action of type TCA_MPLS_ACT_*. */
+};
+
+enum {
+ TCA_MPLS_UNSPEC,
+ TCA_MPLS_TM, /* struct tcf_t; time values associated with action. */
+ TCA_MPLS_PARMS, /* struct tc_mpls; action type and general TC fields. */
+ TCA_MPLS_PAD,
+ TCA_MPLS_PROTO, /* be16; eth_type of pushed or next (for pop) header. */
+ TCA_MPLS_LABEL, /* u32; MPLS label. Lower 20 bits are used. */
+ TCA_MPLS_TC, /* u8; MPLS TC field. Lower 3 bits are used. */
+ TCA_MPLS_TTL, /* u8; MPLS TTL field. Must not be 0. */
+ TCA_MPLS_BOS, /* u8; MPLS BOS field. Either 1 or 0. */
+ __TCA_MPLS_MAX,
+};
+#define TCA_MPLS_MAX (__TCA_MPLS_MAX - 1)
+
+#endif
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 46da15c..34ee236 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -59,6 +59,7 @@
#include <linux/errqueue.h>
#include <linux/prefetch.h>
#include <linux/if_vlan.h>
+#include <linux/mpls.h>
#include <net/protocol.h>
#include <net/dst.h>
@@ -5466,6 +5467,35 @@ int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse)
EXPORT_SYMBOL_GPL(skb_mpls_update_lse);
/**
+ * skb_mpls_dec_ttl() - decrement the TTL of the outermost MPLS header
+ *
+ * @skb: buffer
+ *
+ * Expects skb->data at mac header.
+ *
+ * Returns 0 on success, -errno otherwise.
+ */
+int skb_mpls_dec_ttl(struct sk_buff *skb)
+{
+ u32 lse;
+ u8 ttl;
+
+ if (unlikely(!eth_p_mpls(skb->protocol)))
+ return -EINVAL;
+
+ lse = be32_to_cpu(mpls_hdr(skb)->label_stack_entry);
+ ttl = (lse & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
+ if (!--ttl)
+ return -EINVAL;
+
+ lse &= ~MPLS_LS_TTL_MASK;
+ lse |= ttl << MPLS_LS_TTL_SHIFT;
+
+ return skb_mpls_update_lse(skb, cpu_to_be32(lse));
+}
+EXPORT_SYMBOL_GPL(skb_mpls_dec_ttl);
+
+/**
* alloc_skb_with_frags - allocate skb with page frags
*
* @header_len: size of linear part
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 360fdd3..731f5fb 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -842,6 +842,17 @@ config NET_ACT_CSUM
To compile this code as a module, choose M here: the
module will be called act_csum.
+config NET_ACT_MPLS
+ tristate "MPLS manipulation"
+ depends on NET_CLS_ACT
+ help
+ Say Y here to push or pop MPLS headers.
+
+ If unsure, say N.
+
+ To compile this code as a module, choose M here: the
+ module will be called act_mpls.
+
config NET_ACT_VLAN
tristate "Vlan manipulation"
depends on NET_CLS_ACT
diff --git a/net/sched/Makefile b/net/sched/Makefile
index d54bfcb..c266036 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_NET_ACT_PEDIT) += act_pedit.o
obj-$(CONFIG_NET_ACT_SIMP) += act_simple.o
obj-$(CONFIG_NET_ACT_SKBEDIT) += act_skbedit.o
obj-$(CONFIG_NET_ACT_CSUM) += act_csum.o
+obj-$(CONFIG_NET_ACT_MPLS) += act_mpls.o
obj-$(CONFIG_NET_ACT_VLAN) += act_vlan.o
obj-$(CONFIG_NET_ACT_BPF) += act_bpf.o
obj-$(CONFIG_NET_ACT_CONNMARK) += act_connmark.o
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
new file mode 100644
index 0000000..ca2597c
--- /dev/null
+++ b/net/sched/act_mpls.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mpls.h>
+#include <linux/rtnetlink.h>
+#include <linux/skbuff.h>
+#include <linux/tc_act/tc_mpls.h>
+#include <net/mpls.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
+#include <net/tc_act/tc_mpls.h>
+
+static unsigned int mpls_net_id;
+static struct tc_action_ops act_mpls_ops;
+
+#define ACT_MPLS_TTL_DEFAULT 255
+
+static __be32 tcf_mpls_get_lse(struct mpls_shim_hdr *lse,
+ struct tcf_mpls_params *p, bool set_bos)
+{
+ u32 new_lse = 0;
+
+ if (lse)
+ new_lse = be32_to_cpu(lse->label_stack_entry);
+
+ if (p->tcfm_label != ACT_MPLS_LABEL_NOT_SET) {
+ new_lse &= ~MPLS_LS_LABEL_MASK;
+ new_lse |= p->tcfm_label << MPLS_LS_LABEL_SHIFT;
+ }
+ if (p->tcfm_ttl) {
+ new_lse &= ~MPLS_LS_TTL_MASK;
+ new_lse |= p->tcfm_ttl << MPLS_LS_TTL_SHIFT;
+ }
+ if (p->tcfm_tc != ACT_MPLS_TC_NOT_SET) {
+ new_lse &= ~MPLS_LS_TC_MASK;
+ new_lse |= p->tcfm_tc << MPLS_LS_TC_SHIFT;
+ }
+ if (p->tcfm_bos != ACT_MPLS_BOS_NOT_SET) {
+ new_lse &= ~MPLS_LS_S_MASK;
+ new_lse |= p->tcfm_bos << MPLS_LS_S_SHIFT;
+ } else if (set_bos) {
+ new_lse |= 1 << MPLS_LS_S_SHIFT;
+ }
+
+ return cpu_to_be32(new_lse);
+}
+
+static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a,
+ struct tcf_result *res)
+{
+ struct tcf_mpls *m = to_mpls(a);
+ struct tcf_mpls_params *p;
+ __be32 new_lse;
+ int ret;
+
+ tcf_lastuse_update(&m->tcf_tm);
+ bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
+
+ /* Ensure 'data' points at mac_header prior calling mpls manipulating
+ * functions.
+ */
+ if (skb_at_tc_ingress(skb))
+ skb_push_rcsum(skb, skb->mac_len);
+
+ ret = READ_ONCE(m->tcf_action);
+
+ p = rcu_dereference_bh(m->mpls_p);
+
+ switch (p->tcfm_action) {
+ case TCA_MPLS_ACT_POP:
+ if (skb_mpls_pop(skb, p->tcfm_proto))
+ goto drop;
+ break;
+ case TCA_MPLS_ACT_PUSH:
+ new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb->protocol));
+ if (skb_mpls_push(skb, new_lse, p->tcfm_proto))
+ goto drop;
+ break;
+ case TCA_MPLS_ACT_MODIFY:
+ new_lse = tcf_mpls_get_lse(mpls_hdr(skb), p, false);
+ if (skb_mpls_update_lse(skb, new_lse))
+ goto drop;
+ break;
+ case TCA_MPLS_ACT_DEC_TTL:
+ if (skb_mpls_dec_ttl(skb))
+ goto drop;
+ break;
+ }
+
+ if (skb_at_tc_ingress(skb))
+ skb_pull_rcsum(skb, skb->mac_len);
+
+ return ret;
+
+drop:
+ qstats_drop_inc(this_cpu_ptr(m->common.cpu_qstats));
+ return TC_ACT_SHOT;
+}
+
+static int valid_label(const struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ const u32 *label = nla_data(attr);
+
+ if (*label & ~MPLS_LABEL_MASK || *label == MPLS_LABEL_IMPLNULL) {
+ NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
+ [TCA_MPLS_UNSPEC] = { .strict_start_type = TCA_MPLS_UNSPEC + 1 },
+ [TCA_MPLS_PARMS] = NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
+ [TCA_MPLS_PROTO] = { .type = NLA_U16 },
+ [TCA_MPLS_LABEL] = NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
+ [TCA_MPLS_TC] = NLA_POLICY_RANGE(NLA_U8, 0, 7),
+ [TCA_MPLS_TTL] = NLA_POLICY_MIN(NLA_U8, 1),
+ [TCA_MPLS_BOS] = NLA_POLICY_RANGE(NLA_U8, 0, 1),
+};
+
+static int tcf_mpls_init(struct net *net, struct nlattr *nla,
+ struct nlattr *est, struct tc_action **a,
+ int ovr, int bind, bool rtnl_held,
+ struct tcf_proto *tp, struct netlink_ext_ack *extack)
+{
+ struct tc_action_net *tn = net_generic(net, mpls_net_id);
+ struct nlattr *tb[TCA_MPLS_MAX + 1];
+ struct tcf_chain *goto_ch = NULL;
+ struct tcf_mpls_params *p;
+ struct tc_mpls *parm;
+ bool exists = false;
+ struct tcf_mpls *m;
+ int ret = 0, err;
+ u8 mpls_ttl = 0;
+
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing netlink attributes");
+ return -EINVAL;
+ }
+
+ err = nla_parse_nested(tb, TCA_MPLS_MAX, nla, mpls_policy, extack);
+ if (err < 0)
+ return err;
+
+ if (!tb[TCA_MPLS_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "No MPLS params");
+ return -EINVAL;
+ }
+ parm = nla_data(tb[TCA_MPLS_PARMS]);
+
+ /* Verify parameters against action type. */
+ switch (parm->m_action) {
+ case TCA_MPLS_ACT_POP:
+ if (!tb[TCA_MPLS_PROTO]) {
+ NL_SET_ERR_MSG_MOD(extack, "Protocol must be set for MPLS pop");
+ return -EINVAL;
+ }
+ if (!eth_proto_is_802_3(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid protocol type for MPLS pop");
+ return -EINVAL;
+ }
+ if (tb[TCA_MPLS_LABEL] || tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC] ||
+ tb[TCA_MPLS_BOS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Label, TTL, TC or BOS cannot be used with MPLS pop");
+ return -EINVAL;
+ }
+ break;
+ case TCA_MPLS_ACT_DEC_TTL:
+ if (tb[TCA_MPLS_PROTO] || tb[TCA_MPLS_LABEL] ||
+ tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC] || tb[TCA_MPLS_BOS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Label, TTL, TC, BOS or protocol cannot be used with MPLS dec_ttl");
+ return -EINVAL;
+ }
+ break;
+ case TCA_MPLS_ACT_PUSH:
+ if (!tb[TCA_MPLS_LABEL]) {
+ NL_SET_ERR_MSG_MOD(extack, "Label is required for MPLS push");
+ return -EINVAL;
+ }
+ if (tb[TCA_MPLS_PROTO] &&
+ !eth_p_mpls(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
+ NL_SET_ERR_MSG_MOD(extack, "Protocol must be an MPLS type for MPLS push");
+ return -EPROTONOSUPPORT;
+ }
+ /* Push needs a TTL - if not specified, set a default value. */
+ if (!tb[TCA_MPLS_TTL]) {
+#if IS_ENABLED(CONFIG_MPLS)
+ mpls_ttl = net->mpls.default_ttl ?
+ net->mpls.default_ttl : ACT_MPLS_TTL_DEFAULT;
+#else
+ mpls_ttl = ACT_MPLS_TTL_DEFAULT;
+#endif
+ }
+ break;
+ case TCA_MPLS_ACT_MODIFY:
+ if (tb[TCA_MPLS_PROTO]) {
+ NL_SET_ERR_MSG_MOD(extack, "Protocol cannot be used with MPLS modify");
+ return -EINVAL;
+ }
+ break;
+ default:
+ NL_SET_ERR_MSG_MOD(extack, "Unknown MPLS action");
+ return -EINVAL;
+ }
+
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
+ if (exists && bind)
+ return 0;
+
+ if (!exists) {
+ ret = tcf_idr_create(tn, parm->index, est, a,
+ &act_mpls_ops, bind, true);
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
+ return ret;
+ }
+
+ ret = ACT_P_CREATED;
+ } else if (!ovr) {
+ tcf_idr_release(*a, bind);
+ return -EEXIST;
+ }
+
+ err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
+ if (err < 0)
+ goto release_idr;
+
+ m = to_mpls(*a);
+
+ p = kzalloc(sizeof(*p), GFP_KERNEL);
+ if (!p) {
+ err = -ENOMEM;
+ goto put_chain;
+ }
+
+ p->tcfm_action = parm->m_action;
+ p->tcfm_label = tb[TCA_MPLS_LABEL] ? nla_get_u32(tb[TCA_MPLS_LABEL]) :
+ ACT_MPLS_LABEL_NOT_SET;
+ p->tcfm_tc = tb[TCA_MPLS_TC] ? nla_get_u8(tb[TCA_MPLS_TC]) :
+ ACT_MPLS_TC_NOT_SET;
+ p->tcfm_ttl = tb[TCA_MPLS_TTL] ? nla_get_u8(tb[TCA_MPLS_TTL]) :
+ mpls_ttl;
+ p->tcfm_bos = tb[TCA_MPLS_BOS] ? nla_get_u8(tb[TCA_MPLS_BOS]) :
+ ACT_MPLS_BOS_NOT_SET;
+ p->tcfm_proto = tb[TCA_MPLS_PROTO] ? nla_get_be16(tb[TCA_MPLS_PROTO]) :
+ htons(ETH_P_MPLS_UC);
+
+ spin_lock_bh(&m->tcf_lock);
+ goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
+ rcu_swap_protected(m->mpls_p, p, lockdep_is_held(&m->tcf_lock));
+ spin_unlock_bh(&m->tcf_lock);
+
+ if (goto_ch)
+ tcf_chain_put_by_act(goto_ch);
+ if (p)
+ kfree_rcu(p, rcu);
+
+ if (ret == ACT_P_CREATED)
+ tcf_idr_insert(tn, *a);
+ return ret;
+put_chain:
+ if (goto_ch)
+ tcf_chain_put_by_act(goto_ch);
+release_idr:
+ tcf_idr_release(*a, bind);
+ return err;
+}
+
+static void tcf_mpls_cleanup(struct tc_action *a)
+{
+ struct tcf_mpls *m = to_mpls(a);
+ struct tcf_mpls_params *p;
+
+ p = rcu_dereference_protected(m->mpls_p, 1);
+ if (p)
+ kfree_rcu(p, rcu);
+}
+
+static int tcf_mpls_dump(struct sk_buff *skb, struct tc_action *a,
+ int bind, int ref)
+{
+ unsigned char *b = skb_tail_pointer(skb);
+ struct tcf_mpls *m = to_mpls(a);
+ struct tcf_mpls_params *p;
+ struct tc_mpls opt = {
+ .index = m->tcf_index,
+ .refcnt = refcount_read(&m->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&m->tcf_bindcnt) - bind,
+ };
+ struct tcf_t t;
+
+ spin_lock_bh(&m->tcf_lock);
+ opt.action = m->tcf_action;
+ p = rcu_dereference_protected(m->mpls_p, lockdep_is_held(&m->tcf_lock));
+ opt.m_action = p->tcfm_action;
+
+ if (nla_put(skb, TCA_MPLS_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ if (p->tcfm_label != ACT_MPLS_LABEL_NOT_SET &&
+ nla_put_u32(skb, TCA_MPLS_LABEL, p->tcfm_label))
+ goto nla_put_failure;
+
+ if (p->tcfm_tc != ACT_MPLS_TC_NOT_SET &&
+ nla_put_u8(skb, TCA_MPLS_TC, p->tcfm_tc))
+ goto nla_put_failure;
+
+ if (p->tcfm_ttl && nla_put_u8(skb, TCA_MPLS_TTL, p->tcfm_ttl))
+ goto nla_put_failure;
+
+ if (p->tcfm_bos != ACT_MPLS_BOS_NOT_SET &&
+ nla_put_u8(skb, TCA_MPLS_BOS, p->tcfm_bos))
+ goto nla_put_failure;
+
+ if (nla_put_be16(skb, TCA_MPLS_PROTO, p->tcfm_proto))
+ goto nla_put_failure;
+
+ tcf_tm_dump(&t, &m->tcf_tm);
+
+ if (nla_put_64bit(skb, TCA_MPLS_TM, sizeof(t), &t, TCA_MPLS_PAD))
+ goto nla_put_failure;
+
+ spin_unlock_bh(&m->tcf_lock);
+
+ return skb->len;
+
+nla_put_failure:
+ spin_unlock_bh(&m->tcf_lock);
+ nlmsg_trim(skb, b);
+ return -EMSGSIZE;
+}
+
+static int tcf_mpls_walker(struct net *net, struct sk_buff *skb,
+ struct netlink_callback *cb, int type,
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
+}
+
+static int tcf_mpls_search(struct net *net, struct tc_action **a, u32 index)
+{
+ struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+ return tcf_idr_search(tn, a, index);
+}
+
+static struct tc_action_ops act_mpls_ops = {
+ .kind = "mpls",
+ .id = TCA_ID_MPLS,
+ .owner = THIS_MODULE,
+ .act = tcf_mpls_act,
+ .dump = tcf_mpls_dump,
+ .init = tcf_mpls_init,
+ .cleanup = tcf_mpls_cleanup,
+ .walk = tcf_mpls_walker,
+ .lookup = tcf_mpls_search,
+ .size = sizeof(struct tcf_mpls),
+};
+
+static __net_init int mpls_init_net(struct net *net)
+{
+ struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+ return tc_action_net_init(tn, &act_mpls_ops);
+}
+
+static void __net_exit mpls_exit_net(struct list_head *net_list)
+{
+ tc_action_net_exit(net_list, mpls_net_id);
+}
+
+static struct pernet_operations mpls_net_ops = {
+ .init = mpls_init_net,
+ .exit_batch = mpls_exit_net,
+ .id = &mpls_net_id,
+ .size = sizeof(struct tc_action_net),
+};
+
+static int __init mpls_init_module(void)
+{
+ return tcf_register_action(&act_mpls_ops, &mpls_net_ops);
+}
+
+static void __exit mpls_cleanup_module(void)
+{
+ tcf_unregister_action(&act_mpls_ops, &mpls_net_ops);
+}
+
+module_init(mpls_init_module);
+module_exit(mpls_cleanup_module);
+
+MODULE_AUTHOR("Netronome Systems <oss-drivers@netronome.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MPLS manipulation actions");
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v5 1/5] net: core: move push MPLS functionality from OvS to core helper
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
In-Reply-To: <1562113531-29296-1-git-send-email-john.hurley@netronome.com>
Open vSwitch provides code to push an MPLS header to a packet. In
preparation for supporting this in TC, move the push code to an skb helper
that can be reused.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++
net/openvswitch/actions.c | 31 +++--------------------
3 files changed, 69 insertions(+), 27 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b5d427b..0112256 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3446,6 +3446,7 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len);
int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
int skb_vlan_pop(struct sk_buff *skb);
int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
+int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto);
struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy,
gfp_t gfp);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5323441..f1d1e47 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -66,6 +66,7 @@
#include <net/checksum.h>
#include <net/ip6_checksum.h>
#include <net/xfrm.h>
+#include <net/mpls.h>
#include <linux/uaccess.h>
#include <trace/events/skb.h>
@@ -5326,6 +5327,69 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
}
EXPORT_SYMBOL(skb_vlan_push);
+/* Update the ethertype of hdr and the skb csum value if required. */
+static void skb_mod_eth_type(struct sk_buff *skb, struct ethhdr *hdr,
+ __be16 ethertype)
+{
+ if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ __be16 diff[] = { ~hdr->h_proto, ethertype };
+
+ skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
+ }
+
+ hdr->h_proto = ethertype;
+}
+
+/**
+ * skb_mpls_push() - push a new MPLS header after the mac header
+ *
+ * @skb: buffer
+ * @mpls_lse: MPLS label stack entry to push
+ * @mpls_proto: ethertype of the new MPLS header (expects 0x8847 or 0x8848)
+ *
+ * Expects skb->data at mac header.
+ *
+ * Returns 0 on success, -errno otherwise.
+ */
+int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto)
+{
+ struct mpls_shim_hdr *lse;
+ int err;
+
+ if (unlikely(!eth_p_mpls(mpls_proto)))
+ return -EINVAL;
+
+ /* Networking stack does not allow simultaneous Tunnel and MPLS GSO. */
+ if (skb->encapsulation)
+ return -EINVAL;
+
+ err = skb_cow_head(skb, MPLS_HLEN);
+ if (unlikely(err))
+ return err;
+
+ if (!skb->inner_protocol) {
+ skb_set_inner_network_header(skb, skb->mac_len);
+ skb_set_inner_protocol(skb, skb->protocol);
+ }
+
+ skb_push(skb, MPLS_HLEN);
+ memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb),
+ skb->mac_len);
+ skb_reset_mac_header(skb);
+ skb_set_network_header(skb, skb->mac_len);
+
+ lse = mpls_hdr(skb);
+ lse->label_stack_entry = mpls_lse;
+ skb_postpush_rcsum(skb, lse, MPLS_HLEN);
+
+ if (skb->dev && skb->dev->type == ARPHRD_ETHER)
+ skb_mod_eth_type(skb, eth_hdr(skb), mpls_proto);
+ skb->protocol = mpls_proto;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(skb_mpls_push);
+
/**
* alloc_skb_with_frags - allocate skb with page frags
*
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index bd13146..a9a6c9c 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -175,34 +175,11 @@ static void update_ethertype(struct sk_buff *skb, struct ethhdr *hdr,
static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
const struct ovs_action_push_mpls *mpls)
{
- struct mpls_shim_hdr *new_mpls_lse;
-
- /* Networking stack do not allow simultaneous Tunnel and MPLS GSO. */
- if (skb->encapsulation)
- return -ENOTSUPP;
-
- if (skb_cow_head(skb, MPLS_HLEN) < 0)
- return -ENOMEM;
-
- if (!skb->inner_protocol) {
- skb_set_inner_network_header(skb, skb->mac_len);
- skb_set_inner_protocol(skb, skb->protocol);
- }
-
- skb_push(skb, MPLS_HLEN);
- memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb),
- skb->mac_len);
- skb_reset_mac_header(skb);
- skb_set_network_header(skb, skb->mac_len);
-
- new_mpls_lse = mpls_hdr(skb);
- new_mpls_lse->label_stack_entry = mpls->mpls_lse;
-
- skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
+ int err;
- if (ovs_key_mac_proto(key) == MAC_PROTO_ETHERNET)
- update_ethertype(skb, eth_hdr(skb), mpls->mpls_ethertype);
- skb->protocol = mpls->mpls_ethertype;
+ err = skb_mpls_push(skb, mpls->mpls_lse, mpls->mpls_ethertype);
+ if (err)
+ return err;
invalidate_flow_key(key);
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v5 3/5] net: core: add MPLS update core helper and use in OvS
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
In-Reply-To: <1562113531-29296-1-git-send-email-john.hurley@netronome.com>
Open vSwitch allows the updating of an existing MPLS header on a packet.
In preparation for supporting similar functionality in TC, move this to a
common skb helper function.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 33 +++++++++++++++++++++++++++++++++
net/openvswitch/actions.c | 13 +++----------
3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 89d5c43..1545c4c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3448,6 +3448,7 @@ int skb_vlan_pop(struct sk_buff *skb);
int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto);
int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto);
+int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse);
struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy,
gfp_t gfp);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ce30989..46da15c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5433,6 +5433,39 @@ int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto)
EXPORT_SYMBOL_GPL(skb_mpls_pop);
/**
+ * skb_mpls_update_lse() - modify outermost MPLS header and update csum
+ *
+ * @skb: buffer
+ * @mpls_lse: new MPLS label stack entry to update to
+ *
+ * Expects skb->data at mac header.
+ *
+ * Returns 0 on success, -errno otherwise.
+ */
+int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse)
+{
+ int err;
+
+ if (unlikely(!eth_p_mpls(skb->protocol)))
+ return -EINVAL;
+
+ err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
+ if (unlikely(err))
+ return err;
+
+ if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ __be32 diff[] = { ~mpls_hdr(skb)->label_stack_entry, mpls_lse };
+
+ skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
+ }
+
+ mpls_hdr(skb)->label_stack_entry = mpls_lse;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(skb_mpls_update_lse);
+
+/**
* alloc_skb_with_frags - allocate skb with page frags
*
* @header_len: size of linear part
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 62715bb..3572e11 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -193,19 +193,12 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
__be32 lse;
int err;
- err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
- if (unlikely(err))
- return err;
-
stack = mpls_hdr(skb);
lse = OVS_MASKED(stack->label_stack_entry, *mpls_lse, *mask);
- if (skb->ip_summed == CHECKSUM_COMPLETE) {
- __be32 diff[] = { ~(stack->label_stack_entry), lse };
-
- skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
- }
+ err = skb_mpls_update_lse(skb, lse);
+ if (err)
+ return err;
- stack->label_stack_entry = lse;
flow_key->mpls.top_lse = lse;
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v5 2/5] net: core: move pop MPLS functionality from OvS to core helper
From: John Hurley @ 2019-07-03 0:25 UTC (permalink / raw)
To: netdev
Cc: davem, jiri, xiyou.wangcong, dsahern, willemdebruijn.kernel,
simon.horman, jakub.kicinski, oss-drivers, John Hurley
In-Reply-To: <1562113531-29296-1-git-send-email-john.hurley@netronome.com>
Open vSwitch provides code to pop an MPLS header to a packet. In
preparation for supporting this in TC, move the pop code to an skb helper
that can be reused.
Remove the, now unused, update_ethertype static function from OvS.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 42 ++++++++++++++++++++++++++++++++++++++++++
net/openvswitch/actions.c | 37 ++-----------------------------------
3 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 0112256..89d5c43 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3447,6 +3447,7 @@ int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
int skb_vlan_pop(struct sk_buff *skb);
int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto);
+int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto);
struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy,
gfp_t gfp);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f1d1e47..ce30989 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5391,6 +5391,48 @@ int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto)
EXPORT_SYMBOL_GPL(skb_mpls_push);
/**
+ * skb_mpls_pop() - pop the outermost MPLS header
+ *
+ * @skb: buffer
+ * @next_proto: ethertype of header after popped MPLS header
+ *
+ * Expects skb->data at mac header.
+ *
+ * Returns 0 on success, -errno otherwise.
+ */
+int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto)
+{
+ int err;
+
+ if (unlikely(!eth_p_mpls(skb->protocol)))
+ return -EINVAL;
+
+ err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
+ if (unlikely(err))
+ return err;
+
+ skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
+ memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
+ skb->mac_len);
+
+ __skb_pull(skb, MPLS_HLEN);
+ skb_reset_mac_header(skb);
+ skb_set_network_header(skb, skb->mac_len);
+
+ if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
+ struct ethhdr *hdr;
+
+ /* use mpls_hdr() to get ethertype to account for VLANs. */
+ hdr = (struct ethhdr *)((void *)mpls_hdr(skb) - ETH_HLEN);
+ skb_mod_eth_type(skb, hdr, next_proto);
+ }
+ skb->protocol = next_proto;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(skb_mpls_pop);
+
+/**
* alloc_skb_with_frags - allocate skb with page frags
*
* @header_len: size of linear part
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index a9a6c9c..62715bb 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -160,18 +160,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
struct sw_flow_key *key,
const struct nlattr *attr, int len);
-static void update_ethertype(struct sk_buff *skb, struct ethhdr *hdr,
- __be16 ethertype)
-{
- if (skb->ip_summed == CHECKSUM_COMPLETE) {
- __be16 diff[] = { ~(hdr->h_proto), ethertype };
-
- skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
- }
-
- hdr->h_proto = ethertype;
-}
-
static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
const struct ovs_action_push_mpls *mpls)
{
@@ -190,31 +178,10 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
{
int err;
- err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
- if (unlikely(err))
+ err = skb_mpls_pop(skb, ethertype);
+ if (err)
return err;
- skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
-
- memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
- skb->mac_len);
-
- __skb_pull(skb, MPLS_HLEN);
- skb_reset_mac_header(skb);
- skb_set_network_header(skb, skb->mac_len);
-
- if (ovs_key_mac_proto(key) == MAC_PROTO_ETHERNET) {
- struct ethhdr *hdr;
-
- /* mpls_hdr() is used to locate the ethertype field correctly in the
- * presence of VLAN tags.
- */
- hdr = (struct ethhdr *)((void *)mpls_hdr(skb) - ETH_HLEN);
- update_ethertype(skb, hdr, ethertype);
- }
- if (eth_p_mpls(skb->protocol))
- skb->protocol = ethertype;
-
invalidate_flow_key(key);
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: santosh.shilimkar @ 2019-07-02 22:47 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <74255414-7e5c-e490-4745-9a8b9a73488d@oracle.com>
On 7/2/19 3:12 PM, Gerd Rausch wrote:
> On 02/07/2019 14.18, santosh.shilimkar@oracle.com wrote:
>> On 7/2/19 2:05 PM, Gerd Rausch wrote:
>>> What do you call "RDS_GET_MR" semantics?
>>>
>> Its a blocking socket call. Meaning after this call return to the
>> user, the key must be valid. With async registration that can't be
>> guaranteed.
>>
>
> If the "IB_WR_REG_MR" operation does not complete successfully within
> the given (to-be-discussed?) timeout, "rds_ib_post_reg_frmr" will return
> "-EBUSY".
>
> And that should propagate up the entire stack and make its way into
> "setsockopt" returning "-1" with "errno == EBUSY".
This is an easy case and this doesn't need any waiting since call just
came back without posting work request.
>
> Do you see a problem with this approach?
> Did you observe a situation where this did not work?
>
Calling rds_ib_post_reg_frmr() and looking at return value doesn't
grantee that the work request postred is gping to be successful.
> Are you saying that no timeout, no matter how large, is large enough?
> If that's the case, we can consider turning the "wait_event_timeout"
> into a "wait_event".
>
Yep. Basically till the ceq handler reports successful completion of
reg_mr or inval_mr, mr is not guaranteed to be registered or
invalidated.
>>> Are you suggesting to
>>> a) Not fix this bug right now and wait until some later point in time
>> When did I say that ? I said have you explored alternate approach to
>> fix the issue and if not could you try it out.
>>
>
> Why explore an alternate approach?
> Do you see a problem with the proposed patch (other than the choice of timeout)?
>
Yes the timeout based proceeding isn't safe. wait_event without timeout
would make it guaranteed and give sync like behavior. This is the
behavior with FMR reg and inval calls. If you make that as
wait_event then am fine with the change.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCHv2] selftests/net: skip psock_tpacket test if KALLSYMS was not enabled
From: David Miller @ 2019-07-03 0:18 UTC (permalink / raw)
To: po-hsu.lin; +Cc: shuah, linux-kselftest, netdev, linux-kernel
In-Reply-To: <20190701044031.19451-1-po-hsu.lin@canonical.com>
From: Po-Hsu Lin <po-hsu.lin@canonical.com>
Date: Mon, 1 Jul 2019 12:40:31 +0800
> The psock_tpacket test will need to access /proc/kallsyms, this would
> require the kernel config CONFIG_KALLSYMS to be enabled first.
>
> Apart from adding CONFIG_KALLSYMS to the net/config file here, check the
> file existence to determine if we can run this test will be helpful to
> avoid a false-positive test result when testing it directly with the
> following commad against a kernel that have CONFIG_KALLSYMS disabled:
> make -C tools/testing/selftests TARGETS=net run_tests
>
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool
From: David Miller @ 2019-07-02 21:01 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, linuxppc-dev, bjking1, pradeep, dnbanerg
In-Reply-To: <1561655353-17114-1-git-send-email-tlfalcon@linux.ibm.com>
From: Thomas Falcon <tlfalcon@linux.ibm.com>
Date: Thu, 27 Jun 2019 12:09:13 -0500
> This patch resolves an issue with sensitive bonding modes
> that require valid speed and duplex settings to function
> properly. Currently, the adapter will report that device
> speed and duplex is unknown if the communication link
> with firmware is unavailable. This decision can break LACP
> configurations if the timing is right.
>
> For example, if invalid speeds are reported, the slave
> device's link state is set to a transitional "fail" state
> and the LACP port is disabled. However, if valid speeds
> are reported later but the link state has not been altered,
> the LACP port will remain disabled. If the link state then
> transitions back to "up" from "fail," it results in a state
> such that the slave reports valid speed/duplex and is up,
> but the LACP port will remain disabled.
>
> Workaround this by reporting the last recorded speed
> and duplex settings unless the device has never been
> activated. In that case or when the hypervisor gives
> invalid values, continue to report unknown speed or
> duplex to ethtool.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
Like Andrew, I have my conerns about this.
If the firmware is unavailable, the link is effectively down. So
you should report link down and unknown link parameters.
Bonding and LACP should do the right thing when the firwmare is
reachable again after the migration and the link goes back up.
If bonding/LACP isn't doing that, then the bug is there.
^ permalink raw reply
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ivan Khoronzhuk @ 2019-07-02 21:41 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Jesper Dangaard Brouer, netdev, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190702211536.GA22618@apalos>
On Wed, Jul 03, 2019 at 12:15:36AM +0300, Ilias Apalodimas wrote:
>Hi Jesper,
>Getting late here, i'll respond in detail tomorrow. One point though
>
>[...]
>>
>> This special use-case, seems confined to your driver. And Ilias told me
>> that XDP is not really a performance benefit for this driver as the HW
>> PPS-limit is hit before the XDP and netstack limit. I ask, does it
>> make sense to add XDP to this driver, if it complicates the code for
>> everybody else?
>I think yes. This is a widely used driver on TI embedded devices so having XDP
>to play along is a nice feature. It's also the first and only armv7 we have
>supporting this. Ivan already found a couple of issues due to the 32-bit
>architecture he is trying to fix, i think there's real benefit in having that,
>performance aside.
>I fully agree we should not impact the performance of the API to support a
>special hardware though. I'll have a look on the 2 solutions tomorrow, but the
>general approach on this one should be 'the simpler the better'
>
>Cheers
>/Ilias
BTW even w/o optimization it has close to 300kpps (but with increased number of
descs) on drop which is very close to netsec measurements Ilias sent. But from
what I know there is no h/w limit on cpsw at all that this CPU can serve, so
my assumption it's rather s/w limit. But that's not main here and XDP usage
has not been estimated enough yet in embedded, where hi speed not only benefit
that can be taken from XDP.
I need more clear circumstances to send v6 ...
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and port attribute
From: Jakub Kicinski @ 2019-07-02 23:42 UTC (permalink / raw)
To: Parav Pandit; +Cc: Jiri Pirko, netdev@vger.kernel.org, Saeed Mahameed
In-Reply-To: <AM0PR05MB4866C19C9E6ED767A44C3064D1F80@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 2 Jul 2019 18:50:31 +0000, Parav Pandit wrote:
> > > I didn't see any immediate need to report, at the same time didn't
> > > find any reason to treat such port flavours differently than existing
> > > one. It just gives a clear view of the device's eswitch. Might find it
> > > useful during debugging while inspecting device internal tables..
> >
> > PFs and VFs ports are not tied to network ports in switchdev mode.
> > You have only one network port under a devlink instance AFAIR, anyway.
> >
> I am not sure what do you mean by network port.
DEVLINK_PORT_FLAVOUR_PHYSICAL
> Do you intent to see a physical port that connects to physical network?
>
> As I described in the comment of the PF and VF flavour, it is an eswitch port.
> I have shown the diagram also of the eswitch in the cover letter.
> Port_number doesn't have to a physical port. Flavour describe what
> port type is and number says what is the eswitch port number.
> Hope it clarifies.
I understand what you're doing. If you want to expose some device
specific eswitch port ID please add a new attribute for that.
The fact that that ID may match port_number for your device today
is coincidental. port_number, and split attributes should not be
exposed for PCI ports.
^ permalink raw reply
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Jesper Dangaard Brouer @ 2019-07-02 21:02 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap, brouer
In-Reply-To: <20190702185839.GH4510@khorivan>
On Tue, 2 Jul 2019 21:58:40 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> On Tue, Jul 02, 2019 at 08:29:07PM +0200, Jesper Dangaard Brouer wrote:
> >On Tue, 2 Jul 2019 18:21:13 +0300
> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >
> >> On Tue, Jul 02, 2019 at 05:10:29PM +0200, Jesper Dangaard Brouer wrote:
> >> >On Tue, 2 Jul 2019 17:56:13 +0300
> >> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >> >
> >> >> On Tue, Jul 02, 2019 at 04:52:30PM +0200, Jesper Dangaard Brouer wrote:
> >> >> >On Tue, 2 Jul 2019 17:44:27 +0300
> >> >> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >> >> >
> >> >> >> On Tue, Jul 02, 2019 at 04:31:39PM +0200, Jesper Dangaard Brouer wrote:
> >> >> >> >From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >> >> >
> >> >> >> >Jesper recently removed page_pool_destroy() (from driver invocation) and
> >> >> >> >moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
> >> >> >> >handle in-flight packets/pages. This created an asymmetry in drivers
> >> >> >> >create/destroy pairs.
> >> >> >> >
> >> >> >> >This patch add page_pool user refcnt and reintroduce page_pool_destroy.
> >> >> >> >This serves two purposes, (1) simplify drivers error handling as driver now
> >> >> >> >drivers always calls page_pool_destroy() and don't need to track if
> >> >> >> >xdp_rxq_info_reg_mem_model() was unsuccessful. (2) allow special cases
> >> >> >> >where a single RX-queue (with a single page_pool) provides packets for two
> >> >> >> >net_device'es, and thus needs to register the same page_pool twice with two
> >> >> >> >xdp_rxq_info structures.
> >> >> >>
> >> >> >> As I tend to use xdp level patch there is no more reason to mention (2) case
> >> >> >> here. XDP patch serves it better and can prevent not only obj deletion but also
> >> >> >> pool flush, so, this one patch I could better leave only for (1) case.
> >> >> >
> >> >> >I don't understand what you are saying.
> >> >> >
> >> >> >Do you approve this patch, or do you reject this patch?
> >> >> >
> >> >> It's not reject, it's proposition to use both, XDP and page pool patches,
> >> >> each having its goal.
> >> >
> >> >Just to be clear, if you want this patch to get accepted you have to
> >> >reply with your Signed-off-by (as I wrote).
> >> >
> >> >Maybe we should discuss it in another thread, about why you want two
> >> >solutions to the same problem.
> >>
> >> If it solves same problem I propose to reject this one and use this:
> >> https://lkml.org/lkml/2019/7/2/651
> >
> >No, I propose using this one, and rejecting the other one.
>
> There is at least several arguments against this one (related (2) purpose)
>
> It allows:
> - avoid changes to page_pool/mlx5/netsec
> - save not only allocator obj but allocator "page/buffer flush"
> - buffer flush can be present not only in page_pool but for other allocators
> that can behave differently and not so simple solution.
> - to not limit cpsw/(potentially others) to use "page_pool" allocator only
> ....
>
> This patch better leave also, as it simplifies error path for page_pool and
> have more error prone usage comparing with existent one.
>
> Please, don't limit cpsw and potentially other drivers to use only
> page_pool it can be zca or etc... I don't won't to modify each allocator.
> I propose to add both as by fact they solve different problems with common
> solution.
I'm trying to limit the scope of your changes, for your special case,
because I'm afraid this more common solution is going to limit our
options, painting ourselves into a corner.
E.g. for correct lifetime handling, I think we actually need to do a
dev_hold() on the net_device. (Changes in f71fec47c2 might not be
enough, but I first need to dig into the details and ask Hellwig about
some details). Adding that after your patch is more complicated (if
even doable).
E.g. doing dev_hold() on the net_device, can also turn into a
performance advantage, when/if page_pool is extended to also "travel"
into SKBs. (Allowing to elide such dev_hold() calls in netstack).
I also worry about the possible performance impact these changes will
have down the road. (For the RX/alloc side it should be clear by now
that we gain a lot of performance with the single RX-queue binding and
napi protection). On the return/free side performance *need* to be
improved (it doesn't scale). I'm basically looking at different ways
to bulk return pages into the ptr_ring, which requires changes in
page_pool and likely in xdp_allocator structure. Which your changes
are complicating.
This special use-case, seems confined to your driver. And Ilias told me
that XDP is not really a performance benefit for this driver as the HW
PPS-limit is hit before the XDP and netstack limit. I ask, does it
make sense to add XDP to this driver, if it complicates the code for
everybody else?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH bpf] xdp: fix race on generic receive path
From: Jakub Kicinski @ 2019-07-03 0:40 UTC (permalink / raw)
To: Ilya Maximets
Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jonathan Lemon,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190702143634.19688-1-i.maximets@samsung.com>
On Tue, 2 Jul 2019 17:36:34 +0300, Ilya Maximets wrote:
> Unlike driver mode, generic xdp receive could be triggered
> by different threads on different CPU cores at the same time
> leading to the fill and rx queue breakage. For example, this
> could happen while sending packets from two processes to the
> first interface of veth pair while the second part of it is
> open with AF_XDP socket.
>
> Need to take a lock for each generic receive to avoid race.
>
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a14e8864e4fa..19f41d2b670c 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -119,17 +119,22 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
> {
> u32 metalen = xdp->data - xdp->data_meta;
> u32 len = xdp->data_end - xdp->data;
> + unsigned long flags;
> void *buffer;
> u64 addr;
> int err;
>
> - if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
> - return -EINVAL;
> + spin_lock_irqsave(&xs->rx_lock, flags);
Why _irqsave, rather than _bh?
> + if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index) {
> + err = -EINVAL;
> + goto out_unlock;
> + }
>
> if (!xskq_peek_addr(xs->umem->fq, &addr) ||
> len > xs->umem->chunk_size_nohr - XDP_PACKET_HEADROOM) {
> - xs->rx_dropped++;
> - return -ENOSPC;
> + err = -ENOSPC;
> + goto out_drop;
> }
>
> addr += xs->umem->headroom;
^ permalink raw reply
* Re: [PATCH] net: ethernet: mediatek: Allow non TRGMII mode with MT7621 DDR2 devices
From: David Miller @ 2019-07-02 21:06 UTC (permalink / raw)
To: opensource
Cc: sean.wang, f.fainelli, linux, matthias.bgg, andrew,
vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190629122451.19578-1-opensource@vdorst.com>
From: René van Dorst <opensource@vdorst.com>
Date: Sat, 29 Jun 2019 14:24:51 +0200
> No reason to error out on a MT7621 device with DDR2 memory when non
> TRGMII mode is selected.
> Only MT7621 DDR2 clock setup is not supported for TRGMII mode.
> But non TRGMII mode doesn't need any special clock setup.
>
> Signed-off-by: René van Dorst <opensource@vdorst.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
From: David Miller @ 2019-07-02 22:08 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion
In-Reply-To: <07e0518ac689f5919890a38634df38edf95d34a1.1562000095.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 2 Jul 2019 00:54:55 +0800
> For these places are protected by rcu_read_lock, we change from
> rcu_dereference_rtnl to rcu_dereference, as there is no need to
> check if rtnl lock is held.
>
> For these places are protected by rtnl_lock, we change from
> rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
> as no extra memory barriers are needed under rtnl_lock() which also
> protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
>
> rcu_dereference_rtnl will be only used in the places where it could
> be under rcu_read_lock or rtnl_lock.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
In the cases where RTNL is held, even if rcu_read_lock() is also taken,
we should use rtnl_dereference() because that avoids the READ_ONCE().
^ permalink raw reply
* Re: [PATCH net-next] af_packet: convert pending frame counter to atomic_t
From: David Miller @ 2019-07-02 21:03 UTC (permalink / raw)
To: nhorman; +Cc: netdev, willemb
In-Reply-To: <20190628145206.13871-1-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 28 Jun 2019 10:52:06 -0400
> Given that the socket transmit path is an exclusive path (locked via the
> pg_vec_lock mutex), we do not have the ability to increment this counter
> on multiple cpus in parallel. This implementation also seems to have
> the potential to be broken, in that, should an skb be freed on a cpu
> other than the one that it was initially transmitted on, we may
> decrement a counter that was not initially incremented, leading to
> underflow.
This isn't a problem. There is only one valid "read" operation and that
is the "summation" of all of the per-cpu counters.
All of the overflows and underflows cancel out in the situation you
describe, so all is fine.
I'm hesitant to get behind any fiddling in this area.
Thanks Neil.
^ permalink raw reply
* Re: [PATCH] ipv4: Fix off-by-one in route dump counter without netlink strict checking
From: David Miller @ 2019-07-02 21:07 UTC (permalink / raw)
To: sbrivio; +Cc: dsahern, netdev
In-Reply-To: <74faa085e6af026f8b9f0d3cce8a94147781f257.1561830851.git.sbrivio@redhat.com>
From: Stefano Brivio <sbrivio@redhat.com>
Date: Sat, 29 Jun 2019 19:55:08 +0200
> In commit ee28906fd7a1 ("ipv4: Dump route exceptions if requested") I
> added a counter of per-node dumped routes (including actual routes and
> exceptions), analogous to the existing counter for dumped nodes. Dumping
> exceptions means we need to also keep track of how many routes are dumped
> for each node: this would be just one route per node, without exceptions.
>
> When netlink strict checking is not enabled, we dump both routes and
> exceptions at the same time: the RTM_F_CLONED flag is not used as a
> filter. In this case, the per-node counter 'i_fa' is incremented by one
> to track the single dumped route, then also incremented by one for each
> exception dumped, and then stored as netlink callback argument as skip
> counter, 's_fa', to be used when a partial dump operation restarts.
>
> The per-node counter needs to be increased by one also when we skip a
> route (exception) due to a previous non-zero skip counter, because it
> needs to match the existing skip counter, if we are dumping both routes
> and exceptions. I missed this, and only incremented the counter, for
> regular routes, if the previous skip counter was zero. This means that,
> in case of a mixed dump, partial dump operations after the first one
> will start with a mismatching skip counter value, one less than expected.
>
> This means in turn that the first exception for a given node is skipped
> every time a partial dump operation restarts, if netlink strict checking
> is not enabled (iproute < 5.0).
>
> It turns out I didn't repeat the test in its final version, commit
> de755a85130e ("selftests: pmtu: Introduce list_flush_ipv4_exception test
> case"), which also counts the number of route exceptions returned, with
> iproute2 versions < 5.0 -- I was instead using the equivalent of the IPv6
> test as it was before commit b964641e9925 ("selftests: pmtu: Make
> list_flush_ipv6_exception test more demanding").
>
> Always increment the per-node counter by one if we previously dumped
> a regular route, so that it matches the current skip counter.
>
> Fixes: ee28906fd7a1 ("ipv4: Dump route exceptions if requested")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Applied to net-next, thanks for fixing this.
^ permalink raw reply
* Re: [RFC iproute2] netns: add mounting state file for each netns
From: Stephen Hemminger @ 2019-07-03 0:41 UTC (permalink / raw)
To: Matteo Croce; +Cc: netdev, Alexander Aring, Nicolas Dichtel
In-Reply-To: <20190630192933.30743-1-mcroce@redhat.com>
On Sun, 30 Jun 2019 21:29:33 +0200
Matteo Croce <mcroce@redhat.com> wrote:
> @@ -737,6 +746,14 @@ static int netns_add(int argc, char **argv, bool create)
> }
> close(fd);
>
> + fd = open(tmp_path, O_RDONLY|O_CREAT|O_EXCL, 0);
> + if (fd < 0) {
> + fprintf(stderr, "Cannot create namespace file \"%s\": %s\n",
> + tmp_path, strerror(errno));
> + goto out_delete;
> + }
> + close(fd);
> +
> if (create) {
> netns_save();
> if (unshare(CLONE_NEWNET) < 0) {
> @@ -757,6 +774,7 @@ static int netns_add(int argc, char **argv, bool create)
> goto out_delete;
> }
> netns_restore();
> + unlink(tmp_path);
This looks like yet another source of potential errors and races.
What if the program is killed or other issues.
Maybe using abstract unix domain socket (which doesn't exist in filesystem
and auto-deletes on exit) would be better.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
From: Stanislav Fomichev @ 2019-07-02 22:57 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team
In-Reply-To: <20190529173611.4012579-7-andriin@fb.com>
On 05/29, Andrii Nakryiko wrote:
> 0 is a valid FD, so it's better to initialize it to -1, as is done in
> other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> type), so it's more reliable to check btf_fd, instead of
> btf_key_type_id, to determine if there is any BTF associated with a map.
>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c972fa10271f..a27a0351e595 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> create_attr.key_size = def->key_size;
> create_attr.value_size = def->value_size;
> create_attr.max_entries = def->max_entries;
> - create_attr.btf_fd = 0;
> + create_attr.btf_fd = -1;
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> if (bpf_map_type__is_map_in_map(def->type) &&
> @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
> }
>
> *pfd = bpf_create_map_xattr(&create_attr);
> - if (*pfd < 0 && create_attr.btf_key_type_id) {
> + if (*pfd < 0 && create_attr.btf_fd >= 0) {
> cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> map->name, cp, errno);
> - create_attr.btf_fd = 0;
> + create_attr.btf_fd = -1;
This breaks libbpf compatibility with the older kernels. If the kernel
doesn't know about btf_fd and we set it to -1, then CHECK_ATTR
fails :-(
Any objections to converting BTF retries to bpf_capabilities and then
knowingly passing bft_fd==0 or proper fd?
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> map->btf_key_type_id = 0;
> @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> char *log_buf;
> int ret;
>
> + if (!insns || !insns_cnt)
> + return -EINVAL;
> +
> memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
> load_attr.prog_type = prog->type;
> load_attr.expected_attach_type = prog->expected_attach_type;
> @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> load_attr.license = license;
> load_attr.kern_version = kern_version;
> load_attr.prog_ifindex = prog->prog_ifindex;
> - load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> + load_attr.prog_btf_fd = prog->btf_fd;
> load_attr.func_info = prog->func_info;
> load_attr.func_info_rec_size = prog->func_info_rec_size;
> load_attr.func_info_cnt = prog->func_info_cnt;
> @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> load_attr.line_info_cnt = prog->line_info_cnt;
> load_attr.log_level = prog->log_level;
> load_attr.prog_flags = prog->prog_flags;
> - if (!load_attr.insns || !load_attr.insns_cnt)
> - return -EINVAL;
>
> retry_load:
> log_buf = malloc(log_buf_size);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: David Miller @ 2019-07-02 22:15 UTC (permalink / raw)
To: edumazet
Cc: netdev, eric.dumazet, jsperbeck, jarod, j.vosburgh, vfalico, andy
In-Reply-To: <20190701174851.70293-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 1 Jul 2019 10:48:51 -0700
> A bonding master can be up while best_slave is NULL.
...
> Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: John Sperbeck <jsperbeck@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tipc: remove ub->ubsock checks
From: David Miller @ 2019-07-02 22:13 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion
In-Reply-To: <d59889f395b2c224131046c832fe1a8056209107.1562000239.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 2 Jul 2019 00:57:19 +0800
> Both tipc_udp_enable and tipc_udp_disable are called under rtnl_lock,
> ub->ubsock could never be NULL in tipc_udp_disable and cleanup_bearer,
> so remove the check.
>
> Also remove the one in tipc_udp_enable by adding "free" label.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Looks good, applied.
^ 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