* [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
* [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 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 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 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 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 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] 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 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 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 3/3 v2] macsec: add brackets and indentation after calling macsec_decrypt
From: Andreas Steinmetz @ 2019-07-02 20:50 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca
At this point, skb could only be a valid pointer, so this patch does
not introduce any functional change.
Change:
The patch now only moves the IS_ERR(skb) case under the block where
macsec_decrypt() is called, but not the call to macsec_post_decrypt().
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-07-02 06:31:27.550120145 +0200
+++ b/drivers/net/macsec.c 2019-07-02 06:33:38.637599529 +0200
@@ -1205,17 +1205,18 @@
/* Disabled && !changed text => skip validation */
if (hdr->tci_an & MACSEC_TCI_C ||
- secy->validate_frames != MACSEC_VALIDATE_DISABLED)
+ secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
- if (IS_ERR(skb)) {
- /* the decrypt callback needs the reference */
- if (PTR_ERR(skb) != -EINPROGRESS) {
- macsec_rxsa_put(rx_sa);
- macsec_rxsc_put(rx_sc);
+ if (IS_ERR(skb)) {
+ /* the decrypt callback needs the reference */
+ if (PTR_ERR(skb) != -EINPROGRESS) {
+ macsec_rxsa_put(rx_sa);
+ macsec_rxsc_put(rx_sc);
+ }
+ rcu_read_unlock();
+ return RX_HANDLER_CONSUMED;
}
- rcu_read_unlock();
- return RX_HANDLER_CONSUMED;
}
if (!macsec_post_decrypt(skb, secy, pn))
^ permalink raw reply
* [PATCH net-next 0/3 v2] macsec: a few cleanups in the receive path
From: Andreas Steinmetz @ 2019-07-02 20:49 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca
This patchset removes some unnecessary code in the receive path of the
macsec driver, and re-indents the error handling after calling
macsec_decrypt to make the post-processing clearer.
This is a combined effort of Sabrina Dubroca <sd@queasysnail.net> and me.
Change in 3/3:
The patch now only moves the IS_ERR(skb) case under the block where
macsec_decrypt() is called, but not the call to macsec_post_decrypt().
^ permalink raw reply
* [PATCH net-next 2/2] net/mlx5: Added devlink info callback
From: Saeed Mahameed @ 2019-07-02 23:55 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Shay Agroskin, Saeed Mahameed
In-Reply-To: <20190702235442.1925-1-saeedm@mellanox.com>
From: Shay Agroskin <shayag@mellanox.com>
The callback is invoked using 'devlink dev info <pci>' command and returns
the running and pending firmware version of the HCA and the name of the
kernel driver.
If there is a pending firmware version (a new version is burned but the
HCA still runs with the previous) it is returned as the stored
firmware version. Otherwise, the running version is returned for this
field.
Output example:
$ devlink dev info pci/0000:00:06.0
pci/0000:00:06.0:
driver mlx5_core
versions:
fixed:
fw.psid MT_0000000009
running:
fw.version 16.26.0100
stored:
fw.version 16.26.0100
Signed-off-by: Shay Agroskin <shayag@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../device_drivers/mellanox/mlx5.rst | 19 ++++++
.../net/ethernet/mellanox/mlx5/core/devlink.c | 60 +++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/Documentation/networking/device_drivers/mellanox/mlx5.rst b/Documentation/networking/device_drivers/mellanox/mlx5.rst
index 4eeef2df912f..214325897732 100644
--- a/Documentation/networking/device_drivers/mellanox/mlx5.rst
+++ b/Documentation/networking/device_drivers/mellanox/mlx5.rst
@@ -10,6 +10,7 @@ Contents
========
- `Enabling the driver and kconfig options`_
+- `Devlink info`_
- `Devlink health reporters`_
Enabling the driver and kconfig options
@@ -101,6 +102,24 @@ Enabling the driver and kconfig options
- CONFIG_VXLAN: When chosen, mlx5 vxaln support will be enabled.
- CONFIG_MLXFW: When chosen, mlx5 firmware flashing support will be enabled (via devlink and ethtool).
+Devlink info
+============
+
+The devlink info reports the running and stored firmware versions on device.
+It also prints the device PSID which represents the HCA board type ID.
+
+User command example::
+
+ $ devlink dev info pci/0000:00:06.0
+ pci/0000:00:06.0:
+ driver mlx5_core
+ versions:
+ fixed:
+ fw.psid MT_0000000009
+ running:
+ fw.version 16.26.0100
+ stored:
+ fw.version 16.26.0100
Devlink health reporters
========================
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 1533c657220b..a400f4430c28 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -25,6 +25,65 @@ static int mlx5_devlink_flash_update(struct devlink *devlink,
return mlx5_firmware_flash(dev, fw, extack);
}
+static u8 mlx5_fw_ver_major(u32 version)
+{
+ return (version >> 24) & 0xff;
+}
+
+static u8 mlx5_fw_ver_minor(u32 version)
+{
+ return (version >> 16) & 0xff;
+}
+
+static u16 mlx5_fw_ver_subminor(u32 version)
+{
+ return version & 0xffff;
+}
+
+#define DEVLINK_FW_STRING_LEN 32
+
+static int
+mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ char version_str[DEVLINK_FW_STRING_LEN];
+ u32 running_fw, stored_fw;
+ int err;
+
+ err = devlink_info_driver_name_put(req, DRIVER_NAME);
+ if (err)
+ return err;
+
+ err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
+ if (err)
+ return err;
+
+ err = mlx5_fw_version_query(dev, &running_fw, &stored_fw);
+ if (err)
+ return err;
+
+ snprintf(version_str, sizeof(version_str), "%d.%d.%04d",
+ mlx5_fw_ver_major(running_fw), mlx5_fw_ver_minor(running_fw),
+ mlx5_fw_ver_subminor(running_fw));
+ err = devlink_info_version_running_put(req, "fw.version", version_str);
+ if (err)
+ return err;
+
+ /* no pending version, return running (stored) version */
+ if (stored_fw == 0)
+ stored_fw = running_fw;
+
+ snprintf(version_str, sizeof(version_str), "%d.%d.%04d",
+ mlx5_fw_ver_major(stored_fw), mlx5_fw_ver_minor(stored_fw),
+ mlx5_fw_ver_subminor(stored_fw));
+ err = devlink_info_version_stored_put(req, "fw.version", version_str);
+ if (err)
+ return err;
+
+ return 0;
+}
+
static const struct devlink_ops mlx5_devlink_ops = {
#ifdef CONFIG_MLX5_ESWITCH
.eswitch_mode_set = mlx5_devlink_eswitch_mode_set,
@@ -35,6 +94,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
.eswitch_encap_mode_get = mlx5_devlink_eswitch_encap_mode_get,
#endif
.flash_update = mlx5_devlink_flash_update,
+ .info_get = mlx5_devlink_info_get,
};
struct devlink *mlx5_devlink_alloc(void)
--
2.21.0
^ permalink raw reply related
* [PATCH net-next 0/2] Mellanox, mlx5 devlink versions query
From: Saeed Mahameed @ 2019-07-02 23:55 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Saeed Mahameed
Hi Dave,
This humble 2 patch series from Shay adds the support for devlink fw
versions query to mlx5 driver.
In the first patch we implement the needed fw commands to support this
feature.
In the 2nd patch we implement the devlink callbacks themselves.
I am not sending this as a pull request since i am not sure when my next
pull request is going to be ready, and these two patches are straight
forward net-next patches.
Thanks,
Saeed.
---
Shay Agroskin (2):
net/mlx5: Added fw version query command
net/mlx5: Added devlink info callback
.../device_drivers/mellanox/mlx5.rst | 19 ++
.../net/ethernet/mellanox/mlx5/core/devlink.c | 60 +++++
drivers/net/ethernet/mellanox/mlx5/core/fw.c | 219 ++++++++++++++++--
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +
4 files changed, 280 insertions(+), 20 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH net-next 1/2] net/mlx5: Added fw version query command
From: Saeed Mahameed @ 2019-07-02 23:55 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Shay Agroskin, Saeed Mahameed
In-Reply-To: <20190702235442.1925-1-saeedm@mellanox.com>
From: Shay Agroskin <shayag@mellanox.com>
Using the MCQI and MCQS registers, we query the running and pending
fw version of the HCA.
The MCQS is queried with sequentially increasing component index, until
a component of type BOOT_IMG is found. Querying this component's version
using the MCQI register yields the running and pending fw version of the
HCA.
Querying MCQI for the pending fw version should be done only after
validating that such fw version exists. This is done my checking
'component update state' field in MCQS output.
Signed-off-by: Shay Agroskin <shayag@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/fw.c | 219 ++++++++++++++++--
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +
2 files changed, 201 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw.c b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
index e8fedb307b2c..6f11941fd6cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
@@ -37,6 +37,37 @@
#include "mlx5_core.h"
#include "../../mlxfw/mlxfw.h"
+enum {
+ MCQS_IDENTIFIER_BOOT_IMG = 0x1,
+ MCQS_IDENTIFIER_OEM_NVCONFIG = 0x4,
+ MCQS_IDENTIFIER_MLNX_NVCONFIG = 0x5,
+ MCQS_IDENTIFIER_CS_TOKEN = 0x6,
+ MCQS_IDENTIFIER_DBG_TOKEN = 0x7,
+ MCQS_IDENTIFIER_GEARBOX = 0xA,
+};
+
+enum {
+ MCQS_UPDATE_STATE_IDLE,
+ MCQS_UPDATE_STATE_IN_PROGRESS,
+ MCQS_UPDATE_STATE_APPLIED,
+ MCQS_UPDATE_STATE_ACTIVE,
+ MCQS_UPDATE_STATE_ACTIVE_PENDING_RESET,
+ MCQS_UPDATE_STATE_FAILED,
+ MCQS_UPDATE_STATE_CANCELED,
+ MCQS_UPDATE_STATE_BUSY,
+};
+
+enum {
+ MCQI_INFO_TYPE_CAPABILITIES = 0x0,
+ MCQI_INFO_TYPE_VERSION = 0x1,
+ MCQI_INFO_TYPE_ACTIVATION_METHOD = 0x5,
+};
+
+enum {
+ MCQI_FW_RUNNING_VERSION = 0,
+ MCQI_FW_STORED_VERSION = 1,
+};
+
static int mlx5_cmd_query_adapter(struct mlx5_core_dev *dev, u32 *out,
int outlen)
{
@@ -392,33 +423,49 @@ static int mlx5_reg_mcda_set(struct mlx5_core_dev *dev,
}
static int mlx5_reg_mcqi_query(struct mlx5_core_dev *dev,
- u16 component_index,
- u32 *max_component_size,
- u8 *log_mcda_word_size,
- u16 *mcda_max_write_size)
+ u16 component_index, bool read_pending,
+ u8 info_type, u16 data_size, void *mcqi_data)
{
- u32 out[MLX5_ST_SZ_DW(mcqi_reg) + MLX5_ST_SZ_DW(mcqi_cap)];
- int offset = MLX5_ST_SZ_DW(mcqi_reg);
- u32 in[MLX5_ST_SZ_DW(mcqi_reg)];
+ u32 out[MLX5_ST_SZ_DW(mcqi_reg) + MLX5_UN_SZ_DW(mcqi_reg_data)] = {};
+ u32 in[MLX5_ST_SZ_DW(mcqi_reg)] = {};
+ void *data;
int err;
- memset(in, 0, sizeof(in));
- memset(out, 0, sizeof(out));
-
MLX5_SET(mcqi_reg, in, component_index, component_index);
- MLX5_SET(mcqi_reg, in, data_size, MLX5_ST_SZ_BYTES(mcqi_cap));
+ MLX5_SET(mcqi_reg, in, read_pending_component, read_pending);
+ MLX5_SET(mcqi_reg, in, info_type, info_type);
+ MLX5_SET(mcqi_reg, in, data_size, data_size);
err = mlx5_core_access_reg(dev, in, sizeof(in), out,
- sizeof(out), MLX5_REG_MCQI, 0, 0);
+ MLX5_ST_SZ_BYTES(mcqi_reg) + data_size,
+ MLX5_REG_MCQI, 0, 0);
if (err)
- goto out;
+ return err;
- *max_component_size = MLX5_GET(mcqi_cap, out + offset, max_component_size);
- *log_mcda_word_size = MLX5_GET(mcqi_cap, out + offset, log_mcda_word_size);
- *mcda_max_write_size = MLX5_GET(mcqi_cap, out + offset, mcda_max_write_size);
+ data = MLX5_ADDR_OF(mcqi_reg, out, data);
+ memcpy(mcqi_data, data, data_size);
-out:
- return err;
+ return 0;
+}
+
+static int mlx5_reg_mcqi_caps_query(struct mlx5_core_dev *dev, u16 component_index,
+ u32 *max_component_size, u8 *log_mcda_word_size,
+ u16 *mcda_max_write_size)
+{
+ u32 mcqi_reg[MLX5_ST_SZ_DW(mcqi_cap)] = {};
+ int err;
+
+ err = mlx5_reg_mcqi_query(dev, component_index, 0,
+ MCQI_INFO_TYPE_CAPABILITIES,
+ MLX5_ST_SZ_BYTES(mcqi_cap), mcqi_reg);
+ if (err)
+ return err;
+
+ *max_component_size = MLX5_GET(mcqi_cap, mcqi_reg, max_component_size);
+ *log_mcda_word_size = MLX5_GET(mcqi_cap, mcqi_reg, log_mcda_word_size);
+ *mcda_max_write_size = MLX5_GET(mcqi_cap, mcqi_reg, mcda_max_write_size);
+
+ return 0;
}
struct mlx5_mlxfw_dev {
@@ -434,8 +481,13 @@ static int mlx5_component_query(struct mlxfw_dev *mlxfw_dev,
container_of(mlxfw_dev, struct mlx5_mlxfw_dev, mlxfw_dev);
struct mlx5_core_dev *dev = mlx5_mlxfw_dev->mlx5_core_dev;
- return mlx5_reg_mcqi_query(dev, component_index, p_max_size,
- p_align_bits, p_max_write_size);
+ if (!MLX5_CAP_GEN(dev, mcam_reg) || !MLX5_CAP_MCAM_REG(dev, mcqi)) {
+ mlx5_core_warn(dev, "caps query isn't supported by running FW\n");
+ return -EOPNOTSUPP;
+ }
+
+ return mlx5_reg_mcqi_caps_query(dev, component_index, p_max_size,
+ p_align_bits, p_max_write_size);
}
static int mlx5_fsm_lock(struct mlxfw_dev *mlxfw_dev, u32 *fwhandle)
@@ -575,3 +627,130 @@ int mlx5_firmware_flash(struct mlx5_core_dev *dev,
return mlxfw_firmware_flash(&mlx5_mlxfw_dev.mlxfw_dev,
firmware, extack);
}
+
+static int mlx5_reg_mcqi_version_query(struct mlx5_core_dev *dev,
+ u16 component_index, bool read_pending,
+ u32 *mcqi_version_out)
+{
+ return mlx5_reg_mcqi_query(dev, component_index, read_pending,
+ MCQI_INFO_TYPE_VERSION,
+ MLX5_ST_SZ_BYTES(mcqi_version),
+ mcqi_version_out);
+}
+
+static int mlx5_reg_mcqs_query(struct mlx5_core_dev *dev, u32 *out,
+ u16 component_index)
+{
+ u8 out_sz = MLX5_ST_SZ_BYTES(mcqs_reg);
+ u32 in[MLX5_ST_SZ_DW(mcqs_reg)] = {};
+ int err;
+
+ memset(out, 0, out_sz);
+
+ MLX5_SET(mcqs_reg, in, component_index, component_index);
+
+ err = mlx5_core_access_reg(dev, in, sizeof(in), out,
+ out_sz, MLX5_REG_MCQS, 0, 0);
+ return err;
+}
+
+/* scans component index sequentially, to find the boot img index */
+static int mlx5_get_boot_img_component_index(struct mlx5_core_dev *dev)
+{
+ u32 out[MLX5_ST_SZ_DW(mcqs_reg)] = {};
+ u16 identifier, component_idx = 0;
+ bool quit;
+ int err;
+
+ do {
+ err = mlx5_reg_mcqs_query(dev, out, component_idx);
+ if (err)
+ return err;
+
+ identifier = MLX5_GET(mcqs_reg, out, identifier);
+ quit = !!MLX5_GET(mcqs_reg, out, last_index_flag);
+ quit |= identifier == MCQS_IDENTIFIER_BOOT_IMG;
+ } while (!quit && ++component_idx);
+
+ if (identifier != MCQS_IDENTIFIER_BOOT_IMG) {
+ mlx5_core_warn(dev, "mcqs: can't find boot_img component ix, last scanned idx %d\n",
+ component_idx);
+ return -EOPNOTSUPP;
+ }
+
+ return component_idx;
+}
+
+static int
+mlx5_fw_image_pending(struct mlx5_core_dev *dev,
+ int component_index,
+ bool *pending_version_exists)
+{
+ u32 out[MLX5_ST_SZ_DW(mcqs_reg)];
+ u8 component_update_state;
+ int err;
+
+ err = mlx5_reg_mcqs_query(dev, out, component_index);
+ if (err)
+ return err;
+
+ component_update_state = MLX5_GET(mcqs_reg, out, component_update_state);
+
+ if (component_update_state == MCQS_UPDATE_STATE_IDLE) {
+ *pending_version_exists = false;
+ } else if (component_update_state == MCQS_UPDATE_STATE_ACTIVE_PENDING_RESET) {
+ *pending_version_exists = true;
+ } else {
+ mlx5_core_warn(dev,
+ "mcqs: can't read pending fw version while fw state is %d\n",
+ component_update_state);
+ return -ENODATA;
+ }
+ return 0;
+}
+
+int mlx5_fw_version_query(struct mlx5_core_dev *dev,
+ u32 *running_ver, u32 *pending_ver)
+{
+ u32 reg_mcqi_version[MLX5_ST_SZ_DW(mcqi_version)] = {};
+ bool pending_version_exists;
+ int component_index;
+ int err;
+
+ if (!MLX5_CAP_GEN(dev, mcam_reg) || !MLX5_CAP_MCAM_REG(dev, mcqi) ||
+ !MLX5_CAP_MCAM_REG(dev, mcqs)) {
+ mlx5_core_warn(dev, "fw query isn't supported by the FW\n");
+ return -EOPNOTSUPP;
+ }
+
+ component_index = mlx5_get_boot_img_component_index(dev);
+ if (component_index < 0)
+ return component_index;
+
+ err = mlx5_reg_mcqi_version_query(dev, component_index,
+ MCQI_FW_RUNNING_VERSION,
+ reg_mcqi_version);
+ if (err)
+ return err;
+
+ *running_ver = MLX5_GET(mcqi_version, reg_mcqi_version, version);
+
+ err = mlx5_fw_image_pending(dev, component_index, &pending_version_exists);
+ if (err)
+ return err;
+
+ if (!pending_version_exists) {
+ *pending_ver = 0;
+ return 0;
+ }
+
+ err = mlx5_reg_mcqi_version_query(dev, component_index,
+ MCQI_FW_STORED_VERSION,
+ reg_mcqi_version);
+ if (err)
+ return err;
+
+ *pending_ver = MLX5_GET(mcqi_version, reg_mcqi_version, version);
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index 958769702823..471bbc48bc1f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
@@ -205,6 +205,8 @@ int mlx5_set_mtppse(struct mlx5_core_dev *mdev, u8 pin, u8 arm, u8 mode);
int mlx5_firmware_flash(struct mlx5_core_dev *dev, const struct firmware *fw,
struct netlink_ext_ack *extack);
+int mlx5_fw_version_query(struct mlx5_core_dev *dev,
+ u32 *running_ver, u32 *stored_ver);
void mlx5e_init(void);
void mlx5e_cleanup(void);
--
2.21.0
^ permalink raw reply related
* [PATCH v4 bpf-next] bpf: Add support for fq's EDT to HBM
From: brakmo @ 2019-07-02 22:09 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
v3 -> v4: Removed credit update that is not needed
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 | 168 +++++++++++++++++++++++++++++++++++++
samples/bpf/hbm_kern.h | 40 +++++++--
5 files changed, 231 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..a65b677acdb0
--- /dev/null
+++ b/samples/bpf/hbm_edt_kern.c
@@ -0,0 +1,168 @@
+// 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->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
* 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 21:18 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <697adfba-ac8b-db4d-5819-f4c22ec6c76a@oracle.com>
On 7/2/19 2:05 PM, Gerd Rausch wrote:
> On 02/07/2019 09.49, santosh.shilimkar@oracle.com wrote:
>> On 7/1/19 10:11 PM, Gerd Rausch wrote:
>>> For the registration work-requests there is a benefit to wait a short
>>> amount of time only (the trade-off described in patch #1 of this series).
>>>
>> Actually we should just switch this code to what Avinash has
>> finally made in downstream code. That keeps the RDS_GET_MR
>> semantics and makes sure MR is really valid before handing over
>> the key to userland. There is no need for any timeout
>> for registration case.
>>
>
> 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.
> The purpose of waiting for a IB_WR_REG_MR request to complete
> (inside rds_ib_post_reg_frmr) is in fact to make sure
> the memory region is valid.
>
> Regardless of this being true after a specific time-out,
> or an infinite timeout.
>
> For the non-infinite time-out case, there is a check if the request
> was handled by the firmware.
>
> And if a time-out occurred and the firmware didn't handle the request,
> function "rds_ib_post_reg_frmr" will return -EBUSY.
>
>>> Actually, no:
>>> Socket option RDS_GET_MR wasn't even in the code-path of the
>>> tests I performed:
>>>
>>> It were there RDS_CMSG_RDMA_MAP / RDS_CMSG_RDMA_DEST control
>>> messages that ended up calling '__rds_rdma_map".
>>>
>> What option did you use ? Default option with rds-stress is
>> RDS_GET_MR and hence the question.
>>
>
> Not true!:
Its other way round. Thanks for info so default its using inline
registration instead of explicit call.
> How is socket option RDS_GET_MR special with regards to this proposed fix?
>
>>> I don't understand, please elaborate:
>>> a) Are you saying this issue should not be fixed?
>>> b) Or are you suggesting to replace this fix with a different fix?
>>> If it's the later, please point out what you have in mind.
>>> c) ???
>>>
>> All am saying is the code got changed for good reason and that changed
>> code makes some of these race conditions possibly not applicable.
>
> I don't understand this. Please elaborate.
>
>> So instead of these timeout fixes, am suggesting to use that
>> code as fix. At least test it with those changes and see whats
>> the behavior.
>>
>
> 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.
> b) Use a different fix. If you've got a different fix, please share.
>
I don't but its a review of the fix and possible alternate needs to
be discussed. It is not like take my fix or provide an alternate fix.
> And besides these options, is there anything wrong with this fix
> (other than the discussion of what the timeout value ought to be,
> which we can address)?
>
That timeout is a problem because it doesn't guarantee the failure
of operation since its an asyn operation for registration. Instead
of timing out if you poll the CQ for that operation completion, it
makes it full proof. That is the change Avinash has done iirc and
am requesting to look at that fix.
Other 5 fixes from the series looks fine.
Regards,
Santosh
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH net 2/2] macsec: fix checksumming after decryption
From: Andreas Steinmetz @ 2019-07-02 19:47 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Sabrina Dubroca
In-Reply-To: <CAF=yD-LBRZjns1x9_UrhBYZGX8JNeM+r-cYJV=eyYKDTSG8rBQ@mail.gmail.com>
On Tue, 2019-07-02 at 10:35 -0400, Willem de Bruijn wrote:
> On Tue, Jul 2, 2019 at 12:25 AM Andreas Steinmetz <ast@domdv.de> wrote:
> > On Sun, 2019-06-30 at 21:47 -0400, Willem de Bruijn wrote:
> > > On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <ast@domdv.de>
> > > wrote:
> > > > Fix checksumming after decryption.
> > > >
> > > > Signed-off-by: Andreas Steinmetz <ast@domdv.de>
> > > >
> > > > --- a/drivers/net/macsec.c 2019-06-30 22:14:10.250285314 +0200
> > > > +++ b/drivers/net/macsec.c 2019-06-30 22:15:11.931230417 +0200
> > > > @@ -869,6 +869,7 @@
> > > >
> > > > static void macsec_finalize_skb(struct sk_buff *skb, u8 icv_len,
> > > > u8 hdr_len)
> > > > {
> > > > + skb->ip_summed = CHECKSUM_NONE;
> > > > memmove(skb->data + hdr_len, skb->data, 2 * ETH_ALEN);
> > > > skb_pull(skb, hdr_len);
> > > > pskb_trim_unique(skb, skb->len - icv_len);
> > >
> > > Does this belong in macset_reset_skb?
> >
> > Putting this in macsec_reset_skb would then miss out the "nosci:" part
> > of the RX path in macsec_handle_frame().
>
> It is called on each nskb before calling netif_rx.
>
> It indeed is not called when returning RX_HANDLER_PASS, but that is correct?
This is correct. Packets passed on with RX_HANDLER_PASS are either not for this
driver or the special case of being destined for a KaY and in this case being
the MACsec ethernet protocol and thus not IP, so no checksumming.
^ permalink raw reply
* [PATCH net-next] skbuff: increase verbosity when dumping skb data
From: Willem de Bruijn @ 2019-07-02 19:39 UTC (permalink / raw)
To: netdev
Cc: davem, xiyou.wangcong, herbert, eric.dumazet, saeedm,
Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
skb_warn_bad_offload and netdev_rx_csum_fault trigger on hard to debug
issues. Dump more state and the header.
Optionally dump the entire packet and linear segment. This is required
to debug checksum bugs that may include bytes past skb_tail_pointer().
Both call sites call this function inside a net_ratelimit() block.
Limit full packet log further to a hard limit of can_dump_full (5).
Based on an earlier patch by Cong Wang, see link below.
Link: https://patchwork.ozlabs.org/patch/1000841/
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/skbuff.h | 1 +
net/core/dev.c | 16 ++-----
net/core/skbuff.c | 103 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 108 insertions(+), 12 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b5d427b149c92..48b08549a8b78 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1024,6 +1024,7 @@ static inline bool skb_unref(struct sk_buff *skb)
void skb_release_head_state(struct sk_buff *skb);
void kfree_skb(struct sk_buff *skb);
void kfree_skb_list(struct sk_buff *segs);
+void skb_dump(const char *level, const struct sk_buff *skb, bool full_pkt);
void skb_tx_error(struct sk_buff *skb);
void consume_skb(struct sk_buff *skb);
void __consume_stateless_skb(struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 58529318b3a94..fc676b2610e3c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2900,12 +2900,10 @@ static void skb_warn_bad_offload(const struct sk_buff *skb)
else
name = netdev_name(dev);
}
- WARN(1, "%s: caps=(%pNF, %pNF) len=%d data_len=%d gso_size=%d "
- "gso_type=%d ip_summed=%d\n",
+ skb_dump(KERN_WARNING, skb, false);
+ WARN(1, "%s: caps=(%pNF, %pNF)\n",
name, dev ? &dev->features : &null_features,
- skb->sk ? &skb->sk->sk_route_caps : &null_features,
- skb->len, skb->data_len, skb_shinfo(skb)->gso_size,
- skb_shinfo(skb)->gso_type, skb->ip_summed);
+ skb->sk ? &skb->sk->sk_route_caps : &null_features);
}
/*
@@ -3124,13 +3122,7 @@ void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
{
if (net_ratelimit()) {
pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
- if (dev)
- pr_err("dev features: %pNF\n", &dev->features);
- pr_err("skb len=%u data_len=%u pkt_type=%u gso_size=%u gso_type=%u nr_frags=%u ip_summed=%u csum=%x csum_complete_sw=%d csum_valid=%d csum_level=%u\n",
- skb->len, skb->data_len, skb->pkt_type,
- skb_shinfo(skb)->gso_size, skb_shinfo(skb)->gso_type,
- skb_shinfo(skb)->nr_frags, skb->ip_summed, skb->csum,
- skb->csum_complete_sw, skb->csum_valid, skb->csum_level);
+ skb_dump(KERN_ERR, skb, true);
dump_stack();
}
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5323441a12ccf..5d501066d00ca 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -707,6 +707,109 @@ void kfree_skb_list(struct sk_buff *segs)
}
EXPORT_SYMBOL(kfree_skb_list);
+/* Dump skb information and contents.
+ *
+ * Must only be called from net_ratelimit()-ed paths.
+ *
+ * Dumps up to can_dump_full whole packets if full_pkt, headers otherwise.
+ */
+void skb_dump(const char *level, const struct sk_buff *skb, bool full_pkt)
+{
+ static atomic_t can_dump_full = ATOMIC_INIT(5);
+ struct skb_shared_info *sh = skb_shinfo(skb);
+ struct net_device *dev = skb->dev;
+ struct sock *sk = skb->sk;
+ struct sk_buff *list_skb;
+ bool has_mac, has_trans;
+ int headroom, tailroom;
+ int i, len, seg_len;
+
+ if (full_pkt)
+ full_pkt = atomic_dec_if_positive(&can_dump_full) >= 0;
+
+ if (full_pkt)
+ len = skb->len;
+ else
+ len = min_t(int, skb->len, MAX_HEADER + 128);
+
+ headroom = skb_headroom(skb);
+ tailroom = skb_tailroom(skb);
+
+ has_mac = skb_mac_header_was_set(skb);
+ has_trans = skb_transport_header_was_set(skb);
+
+ printk("%sskb len=%u headroom=%u headlen=%u tailroom=%u\n"
+ "mac=(%d,%d) net=(%d,%d) trans=%d\n"
+ "shinfo(txflags=%u nr_frags=%u gso(size=%hu type=%u segs=%hu))\n"
+ "csum(0x%x ip_summed=%u complete_sw=%u valid=%u level=%u)\n"
+ "hash(0x%x sw=%u l4=%u) proto=0x%04x pkttype=%u iif=%d\n",
+ level, skb->len, headroom, skb_headlen(skb), tailroom,
+ has_mac ? skb->mac_header : -1,
+ has_mac ? skb_mac_header_len(skb) : -1,
+ skb->network_header,
+ has_trans ? skb_network_header_len(skb) : -1,
+ has_trans ? skb->transport_header : -1,
+ sh->tx_flags, sh->nr_frags,
+ sh->gso_size, sh->gso_type, sh->gso_segs,
+ skb->csum, skb->ip_summed, skb->csum_complete_sw,
+ skb->csum_valid, skb->csum_level,
+ skb->hash, skb->sw_hash, skb->l4_hash,
+ ntohs(skb->protocol), skb->pkt_type, skb->skb_iif);
+
+ if (dev)
+ printk("%sdev name=%s feat=0x%pNF\n",
+ level, dev->name, &dev->features);
+ if (sk)
+ printk("%ssk family=%hu type=%hu proto=%hu\n",
+ level, sk->sk_family, sk->sk_type, sk->sk_protocol);
+
+ if (full_pkt && headroom)
+ print_hex_dump(level, "skb headroom: ", DUMP_PREFIX_OFFSET,
+ 16, 1, skb->head, headroom, false);
+
+ seg_len = min_t(int, skb_headlen(skb), len);
+ if (seg_len)
+ print_hex_dump(level, "skb linear: ", DUMP_PREFIX_OFFSET,
+ 16, 1, skb->data, seg_len, false);
+ len -= seg_len;
+
+ if (full_pkt && tailroom)
+ print_hex_dump(level, "skb tailroom: ", DUMP_PREFIX_OFFSET,
+ 16, 1, skb_tail_pointer(skb), tailroom, false);
+
+ for (i = 0; len && i < skb_shinfo(skb)->nr_frags; i++) {
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ u32 p_off, p_len, copied;
+ struct page *p;
+ u8 *vaddr;
+
+ skb_frag_foreach_page(frag, frag->page_offset,
+ skb_frag_size(frag), p, p_off, p_len,
+ copied) {
+ seg_len = min_t(int, p_len, len);
+ vaddr = kmap_atomic(p);
+ print_hex_dump(level, "skb frag: ",
+ DUMP_PREFIX_OFFSET,
+ 16, 1, vaddr + p_off, seg_len, false);
+ kunmap_atomic(vaddr);
+ len -= seg_len;
+ if (!len)
+ break;
+ }
+ }
+
+ if (len && skb_has_frag_list(skb)) {
+ printk("skb fraglist:\n");
+ skb_walk_frags(skb, list_skb) {
+ if (len <= 0)
+ break;
+ skb_dump(level, list_skb, len);
+ len -= list_skb->len;
+ }
+ }
+}
+EXPORT_SYMBOL(skb_dump);
+
/**
* skb_tx_error - report an sk_buff xmit error
* @skb: buffer that triggered an error
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: veth pair ping fail if one of them enslaved into a VRF
From: Zoltán Elek @ 2019-07-02 19:36 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, dsa
In-Reply-To: <20190702192122.GA16784@splinter>
Thank you for the detailed answer, I tried your solution and it works
as expected.
Ido Schimmel <idosch@idosch.org> ezt írta (időpont: 2019. júl. 2., K, 21:21):
>
> On Tue, Jul 02, 2019 at 08:42:15PM +0200, Zoltán Elek wrote:
> > Hi!
> >
> > I have a simple scenario, with a veth pair, IP addresses assigned from
> > the same subnet. They can ping eachother. But when I put one of them
> > into a VRF (in the example below, I put veth in-vrf into the test-vrf
> > VRF) the ping fails. My first question: that is the expected behavior?
> > And my second question: is there any way to overcome this?
> >
> > Here are my test commands:
> > ip link add out-of-vrf type veth peer name in-vrf
> > ip link set dev out-of-vrf up
> > ip link set dev in-vrf up
> > ip link add test-vrf type vrf table 10
> > ip link set dev test-vrf up
> > ip -4 addr add 100.127.253.2/24 dev in-vrf
> > ip -4 addr add 100.127.253.1/24 dev out-of-vrf
> >
> > Then ping works as expected:
> > ping -c1 -I 100.127.253.1 100.127.253.2
> >
> > After I put the in-vrf into test-vrf, ping fails:
> > ip link set in-vrf vrf test-vrf up
>
> You need to re-order the FIB rules so that lookup for 100.127.253.1
> happens in table 10 and not in the local table:
>
> # ip -4 rule add pref 32765 table local
> # ip -4 rule del pref 0
> # ip -4 rule show
> 1000: from all lookup [l3mdev-table]
> 32765: from all lookup local
> 32766: from all lookup main
> 32767: from all lookup default
>
> Bad:
>
> ping 16735 [001] 13726.398115: fib:fib_table_lookup: table 255 oif 0 iif
> 9 proto 0 100.127.253.2/0 -> 100.127.253.1/0 tos 0 scope 0 flags 4 ==>
> dev out-of-vrf gw 0.0.0.0 src 100.127.253.1 err 0
>
> Good:
>
> ping 16665 [001] 13500.937145: fib:fib_table_lookup: table 10 oif 0 iif
> 9 proto 0 100.127.253.2/0 -> 100.127.253.1/0 tos 0 scope 0 flags 4 ==>
> dev in-vrf gw 0.0.0.0 src 100.127.253.2 err 0
>
> >
> > Thanks,
> > Zoltan Elek,
> > VI1
^ permalink raw reply
* Re: [PATCH next] sysctl: add proc_dointvec_jiffies_minmax to limit the min/max write value
From: Luis Chamberlain @ 2019-07-02 19:32 UTC (permalink / raw)
To: Zhiqiang Liu
Cc: Kees Cook, akpm, linux-kernel, linux-fsdevel, ebiederm, pbonzini,
viro, adobriyan, mingfangsen, wangxiaogang3, Zhoukang (A), netdev
In-Reply-To: <dd40ae3b-8e0a-2d55-d402-6f261a6c0e09@huawei.com>
On Tue, Jun 04, 2019 at 11:27:51PM +0800, Zhiqiang Liu wrote:
> > On Wed, May 15, 2019 at 10:53:55PM +0800, Zhiqiang Liu wrote:
> >>>> In proc_dointvec_jiffies func, the write value is only checked
> >>>> whether it is larger than INT_MAX. If the write value is less
> >>>> than zero, it can also be successfully writen in the data.
> >
> > This appears to be "be design", but I see many "unsigned int" users
> > that might be tricked into giant values... (for example, see
> > net/netfilter/nf_conntrack_standalone.c)
> >
> > Should proc_dointvec_jiffies() just be fixed to disallow negative values
> > entirely? Looking at the implementation, it seems to be very intentional
> > about accepting negative values.
> >
> > However, when I looked through a handful of proc_dointvec_jiffies()
> > users, it looks like they're all expecting a positive value. Many in the
> > networking subsystem are, in fact, writing to unsigned long variables,
> > as I mentioned.
> >
> I totally agree with you. And I also cannot find an scenario that expects
> negative values. Consideing the "negative" scenario may be exist, I add the
> proc_dointvec_jiffies_minmax like proc_dointvec_minmax.
If no negative values exist, and there is no real point to it, then just
rename the existing one and update the docs.
Luis
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] libbpf: capture value in BTF type info for BTF-defined map defs
From: Andrii Nakryiko @ 2019-07-02 19:29 UTC (permalink / raw)
To: Y Song
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf, netdev,
Song Liu
In-Reply-To: <CAH3MdRXm9EnP-jqLQb-1o7KhtE2Dd3QWhPDF3SHou_At19u-CQ@mail.gmail.com>
On Tue, Jul 2, 2019 at 12:19 PM Y Song <ys114321@gmail.com> wrote:
>
> On Fri, Jun 28, 2019 at 8:26 AM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > Change BTF-defined map definitions to capture compile-time integer
> > values as part of BTF type definition, to avoid split of key/value type
> > information and actual type/size/flags initialization for maps.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++----------------------
> > 1 file changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6e6ebef11ba3..9e099ecb2c2b 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1028,40 +1028,40 @@ static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
> > }
> > }
> >
> > -static bool get_map_field_int(const char *map_name,
> > - const struct btf *btf,
> > +/*
> > + * Fetch integer attribute of BTF map definition. Such attributes are
> > + * represented using a pointer to an array, in which dimensionality of array
> > + * encodes specified integer value. E.g., int (*type)[BPF_MAP_TYPE_ARRAY];
> > + * encodes `type => BPF_MAP_TYPE_ARRAY` key/value pair completely using BTF
> > + * type definition, while using only sizeof(void *) space in ELF data section.
> > + */
> > +static bool get_map_field_int(const char *map_name, const struct btf *btf,
> > const struct btf_type *def,
> > - const struct btf_member *m,
> > - const void *data, __u32 *res) {
> > + const struct btf_member *m, __u32 *res) {
> > const struct btf_type *t = skip_mods_and_typedefs(btf, m->type);
> > const char *name = btf__name_by_offset(btf, m->name_off);
> > - __u32 int_info = *(const __u32 *)(const void *)(t + 1);
> > + const struct btf_array *arr_info;
> > + const struct btf_type *arr_t;
> >
> > - if (BTF_INFO_KIND(t->info) != BTF_KIND_INT) {
> > - pr_warning("map '%s': attr '%s': expected INT, got %u.\n",
> > + if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR) {
> > + pr_warning("map '%s': attr '%s': expected PTR, got %u.\n",
> > map_name, name, BTF_INFO_KIND(t->info));
> > return false;
> > }
> > - if (t->size != 4 || BTF_INT_BITS(int_info) != 32 ||
> > - BTF_INT_OFFSET(int_info)) {
> > - pr_warning("map '%s': attr '%s': expected 32-bit non-bitfield integer, "
> > - "got %u-byte (%d-bit) one with bit offset %d.\n",
> > - map_name, name, t->size, BTF_INT_BITS(int_info),
> > - BTF_INT_OFFSET(int_info));
> > - return false;
> > - }
> > - if (BTF_INFO_KFLAG(def->info) && BTF_MEMBER_BITFIELD_SIZE(m->offset)) {
> > - pr_warning("map '%s': attr '%s': bitfield is not supported.\n",
> > - map_name, name);
> > +
> > + arr_t = btf__type_by_id(btf, t->type);
> > + if (!arr_t) {
> > + pr_warning("map '%s': attr '%s': type [%u] not found.\n",
> > + map_name, name, t->type);
> > return false;
> > }
> > - if (m->offset % 32) {
> > - pr_warning("map '%s': attr '%s': unaligned fields are not supported.\n",
> > - map_name, name);
> > + if (BTF_INFO_KIND(arr_t->info) != BTF_KIND_ARRAY) {
> > + pr_warning("map '%s': attr '%s': expected ARRAY, got %u.\n",
> > + map_name, name, BTF_INFO_KIND(arr_t->info));
> > return false;
> > }
> > -
> > - *res = *(const __u32 *)(data + m->offset / 8);
> > + arr_info = (const void *)(arr_t + 1);
> > + *res = arr_info->nelems;
>
> Here, we use number of array elements (__u32 type) as the value.
>
> But we have
> #define __int(name, val) int (*name)[val]
> which suggests that "val" have type "int".
>
> Do you think using __uint(name, val) may be more consistent?
> If this is something to be enforced, it may be worthwhile to check
> array element type should be __uint, just in case that in the
> future we have different array element type (e.g., int)
> for different purpose.
Currently we don't care about array element type at all. It seems
highly unlikely that we'll use array element type as some sort of
flag, as it would be **way** to much unobvious magic. I'm a bit
worried that there could be some inconsistencies in how compiler emits
or omits SIGNED flag on BTF_KIND_INT, so I'd rather minimize amount of
possible confusion here and not enforce that.
As for __uint vs __int, you are right, the value we can encode is
always unsigned, so it might be a good idea to do __uint instead, I'll
post an updated version.
>
> > return true;
> > }
> >
> > @@ -1074,7 +1074,6 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> > const struct btf_var_secinfo *vi;
> > const struct btf_var *var_extra;
> > const struct btf_member *m;
> > - const void *def_data;
> > const char *map_name;
> > struct bpf_map *map;
> > int vlen, i;
> > @@ -1131,7 +1130,6 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> > pr_debug("map '%s': at sec_idx %d, offset %zu.\n",
> > map_name, map->sec_idx, map->sec_offset);
> >
> > - def_data = data->d_buf + vi->offset;
> > vlen = BTF_INFO_VLEN(def->info);
> > m = (const void *)(def + 1);
> > for (i = 0; i < vlen; i++, m++) {
> > @@ -1144,19 +1142,19 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> > }
> > if (strcmp(name, "type") == 0) {
> > if (!get_map_field_int(map_name, obj->btf, def, m,
> > - def_data, &map->def.type))
> > + &map->def.type))
> > return -EINVAL;
> > pr_debug("map '%s': found type = %u.\n",
> > map_name, map->def.type);
> > } else if (strcmp(name, "max_entries") == 0) {
> > if (!get_map_field_int(map_name, obj->btf, def, m,
> > - def_data, &map->def.max_entries))
> > + &map->def.max_entries))
> > return -EINVAL;
> > pr_debug("map '%s': found max_entries = %u.\n",
> > map_name, map->def.max_entries);
> > } else if (strcmp(name, "map_flags") == 0) {
> > if (!get_map_field_int(map_name, obj->btf, def, m,
> > - def_data, &map->def.map_flags))
> > + &map->def.map_flags))
> > return -EINVAL;
> > pr_debug("map '%s': found map_flags = %u.\n",
> > map_name, map->def.map_flags);
> > @@ -1164,7 +1162,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> > __u32 sz;
> >
> > if (!get_map_field_int(map_name, obj->btf, def, m,
> > - def_data, &sz))
> > + &sz))
> > return -EINVAL;
> > pr_debug("map '%s': found key_size = %u.\n",
> > map_name, sz);
> > @@ -1207,7 +1205,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> > __u32 sz;
> >
> > if (!get_map_field_int(map_name, obj->btf, def, m,
> > - def_data, &sz))
> > + &sz))
> > return -EINVAL;
> > pr_debug("map '%s': found value_size = %u.\n",
> > map_name, sz);
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf: Add support for fq's EDT to HBM
From: Lawrence Brakmo @ 2019-07-02 19:26 UTC (permalink / raw)
To: Yonghong Song, netdev
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eric Dumazet, Kernel Team
In-Reply-To: <d4b43599-3901-920d-377e-c6f2589ac622@fb.com>
Yonhhong, thank you for the feedbac. Repies inlined. I will submit a new revision later today.
On 7/1/19, 2:04 PM, "Yonghong Song" <yhs@fb.com> wrote:
On 6/28/19 12:41 PM, brakmo wrote:
> 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
>
> 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 | 171 +++++++++++++++++++++++++++++++++++++
> samples/bpf/hbm_kern.h | 40 +++++++--
> 5 files changed, 234 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..004a44a83e1e
> --- /dev/null
> +++ b/samples/bpf/hbm_edt_kern.c
> @@ -0,0 +1,171 @@
> +// 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)
> +{
> + signed long long delta = 0, delta_send;
"signed" is not needed here.
Removed
> + 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);
> + if (qsp != NULL && !qsp->loopback && (skb->ifindex == 1))
The skb->ifindex = 1 is skipped. Why? Some comments will
be helpful to understand.
Added comment
> + 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);
> + 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;
> + bpf_printk("Updating rate: %d (1sec:%llu bits)\n",
> + (int)qdp->rate,
> + CREDIT_PER_NS(1000000000, qdp->rate) * 8);
How often this bpf_printk will fire at runtime?
If it fires relatively frequently, maybe worth to put it
under debug?
Removed
> + }
> +
> + // 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);
We have race here. Updating qdp->lasttime may happen concurrently with
the update in bpf_spin_lock region. Do we have any issue here?
Good catch. Should not be an issue as long as both updates are concurrent. I changed the update within the spin_lock_region to also be atomic.
> + 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,
>
^ permalink raw reply
* Re: [PATCH net-next 1/1] qed: Add support for Timestamping the unicast PTP packets.
From: David Miller @ 2019-07-02 19:23 UTC (permalink / raw)
To: skalluru; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190702150412.31132-1-skalluru@marvell.com>
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: Tue, 2 Jul 2019 08:04:12 -0700
> The register definition in the header file captures more details on
> the individual bits.
Where is this register definition in the header file and why aren't
CPP defines from there being used instead of a mask constant mask?
Thanks.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andrii Nakryiko @ 2019-07-02 19:22 UTC (permalink / raw)
To: Song Liu
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team,
Lorenz Bauer, Jann Horn, gregkh
In-Reply-To: <20190627201923.2589391-2-songliubraving@fb.com>
On Thu, Jun 27, 2019 at 1:20 PM Song Liu <songliubraving@fb.com> wrote:
>
> This patch introduce unprivileged BPF access. The access control is
> achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> to call sys_bpf().
>
> Two ioctl command are added to /dev/bpf:
>
> The two commands enable/disable permission to call sys_bpf() for current
> task. This permission is noted by bpf_permitted in task_struct. This
> permission is inherited during clone(CLONE_THREAD).
>
> Helper function bpf_capable() is added to check whether the task has got
> permission via /dev/bpf.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> Documentation/ioctl/ioctl-number.txt | 1 +
> include/linux/bpf.h | 11 +++++
> include/linux/sched.h | 3 ++
> include/uapi/linux/bpf.h | 6 +++
> kernel/bpf/arraymap.c | 2 +-
> kernel/bpf/cgroup.c | 2 +-
> kernel/bpf/core.c | 4 +-
> kernel/bpf/cpumap.c | 2 +-
> kernel/bpf/devmap.c | 2 +-
> kernel/bpf/hashtab.c | 4 +-
> kernel/bpf/lpm_trie.c | 2 +-
> kernel/bpf/offload.c | 2 +-
> kernel/bpf/queue_stack_maps.c | 2 +-
> kernel/bpf/reuseport_array.c | 2 +-
> kernel/bpf/stackmap.c | 2 +-
> kernel/bpf/syscall.c | 71 +++++++++++++++++++++-------
> kernel/bpf/verifier.c | 2 +-
> kernel/bpf/xskmap.c | 2 +-
> kernel/fork.c | 5 ++
> net/core/filter.c | 6 +--
> 20 files changed, 99 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index c9558146ac58..19998b99d603 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -327,6 +327,7 @@ Code Seq#(hex) Include File Comments
> 0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
> 0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
> 0xB6 all linux/fpga-dfl.h
> +0xBP 01-02 uapi/linux/bpf.h <mailto:bpf@vger.kernel.org>
should this be 0xBF?
> 0xC0 00-0F linux/usb/iowarrior.h
> 0xCA 00-0F uapi/misc/cxl.h
> 0xCA 10-2F uapi/misc/ocxl.h
<snip>
^ 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