[ovs-dev] [PATCH] ovs-tcpdump: Fix an undefined variable
From: Hyong Youb Kim
Run ovs-tcpdump without --span, and it throws the following
exception. Define mirror_select_all to avoid the error.
Traceback (most recent call last):
File "/usr/local/bin/ovs-tcpdump", line 488, in
main()
File "/usr/local/bin/ovs-tcpdump", line 454, in main
mirror_select_all)
UnboundLocalError: local variable 'mirror_select_all' referenced before
assignment
Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.")
Signed-off-by: Hyong Youb Kim
Acked-by: Ilya Maximets
---
v3:
* resend with an explicit From: in the email body to avoid checkpatch warning
v2:
* fix a typo: with -> without
* resend after subscribing to dev to avoid ovs-dev in From:
Well, this did not work.
utilities/ovs-tcpdump.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index 22f249f58..269c252f8 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -379,6 +379,7 @@ def main():
skip_next = False
mirror_interface = None
+mirror_select_all = False
dump_cmd = 'tcpdump'
for cur, nxt in argv_tuples(sys.argv[1:]):
--
2.16.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v3 2/2] conntrack: Exclude l2 padding in 'conn_key_extract()'.
> > 'conn_key_extract()' in userspace conntrack is including L2 > (Ethernet) pad bytes for both L3 and L4 sizes. One problem is any packet with > non-zero L2 padding can incorrectly fail L4 checksum validation. > > This patch fixes conn_key_extract() by ignoring L2 pad bytes. > Thanks Darrell for the patch. It looks fine. Can we get this in for upcoming OVS release ? Also need a backport till OVS branch v2.6 if possible. Warm Regards, Vishal Ajmera ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v2] conntrack: Fix possible uninitialized memory.
There are a few cases where padding may be undefined according to
the C standard. Practically, it seems implementations don't have issue,
but it is better to be safe. The code paths modified are not hot ones.
Found by inspection.
Signed-off-by: Darrell Ball
---
v2: Found another one; will double check for others later.
lib/conntrack.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index e1f4041..e7033a8 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -747,6 +747,7 @@ static struct conn *
conn_lookup(struct conntrack *ct, const struct conn_key *key, long long now)
{
struct conn_lookup_ctx ctx;
+memset(, 0, sizeof ctx);
ctx.conn = NULL;
ctx.key = *key;
ctx.hash = conn_key_hash(key, ct->hash_basis);
@@ -896,6 +897,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
if (nat_action_info) {
nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info);
+memset(conn_for_un_nat_copy, 0, sizeof *conn_for_un_nat_copy);
if (alg_exp) {
if (alg_exp->nat_rpl_dst) {
@@ -934,8 +936,6 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
ct_rwlock_unlock(>resources_lock);
}
conn_for_un_nat_copy->conn_type = CT_CONN_TYPE_UN_NAT;
-conn_for_un_nat_copy->nat_info = NULL;
-conn_for_un_nat_copy->alg = NULL;
nat_packet(pkt, nc, ctx->icmp_related);
}
hmap_insert(>buckets[bucket].connections, >node, ctx->hash);
@@ -1262,6 +1262,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
ct->hash_basis,
alg_src_ip_wc(ct_alg_ctl));
if (alg_exp) {
+memset(_exp_entry, 0, sizeof alg_exp_entry);
alg_exp_entry = *alg_exp;
alg_exp = _exp_entry;
}
@@ -2024,7 +2025,9 @@ conn_key_hash(const struct conn_key *key, uint32_t basis)
static void
conn_key_reverse(struct conn_key *key)
{
-struct ct_endpoint tmp = key->src;
+struct ct_endpoint tmp;
+memset(, 0, sizeof tmp);
+tmp = key->src;
key->src = key->dst;
key->dst = tmp;
}
@@ -2614,7 +2617,9 @@ static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key,
uint32_t basis, bool src_ip_wc)
{
-struct conn_key check_key = *key;
+struct conn_key check_key;
+memset(_key, 0, sizeof check_key);
+check_key = *key;
check_key.src.port = ALG_WC_SRC_PORT;
if (src_ip_wc) {
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v1] conntrack: Fix possible uninitialized memory.
There are a few cases where padding may be undefined according to
the C standard. Practically, it seems implementations don't have issue,
but it is better to be safe. The code paths modified are not hot ones.
Found by inspection.
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index e1f4041..a379eaa 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -747,6 +747,7 @@ static struct conn *
conn_lookup(struct conntrack *ct, const struct conn_key *key, long long now)
{
struct conn_lookup_ctx ctx;
+memset(, 0, sizeof ctx);
ctx.conn = NULL;
ctx.key = *key;
ctx.hash = conn_key_hash(key, ct->hash_basis);
@@ -896,6 +897,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
if (nat_action_info) {
nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info);
+memset(conn_for_un_nat_copy, 0, sizeof *conn_for_un_nat_copy);
if (alg_exp) {
if (alg_exp->nat_rpl_dst) {
@@ -934,8 +936,6 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
ct_rwlock_unlock(>resources_lock);
}
conn_for_un_nat_copy->conn_type = CT_CONN_TYPE_UN_NAT;
-conn_for_un_nat_copy->nat_info = NULL;
-conn_for_un_nat_copy->alg = NULL;
nat_packet(pkt, nc, ctx->icmp_related);
}
hmap_insert(>buckets[bucket].connections, >node, ctx->hash);
@@ -2024,7 +2024,9 @@ conn_key_hash(const struct conn_key *key, uint32_t basis)
static void
conn_key_reverse(struct conn_key *key)
{
-struct ct_endpoint tmp = key->src;
+struct ct_endpoint tmp;
+memset(, 0, sizeof tmp);
+tmp = key->src;
key->src = key->dst;
key->dst = tmp;
}
@@ -2614,7 +2616,9 @@ static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key,
uint32_t basis, bool src_ip_wc)
{
-struct conn_key check_key = *key;
+struct conn_key check_key;
+memset(_key, 0, sizeof check_key);
+check_key = *key;
check_key.src.port = ALG_WC_SRC_PORT;
if (src_ip_wc) {
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofp-actions: Set an action depth limit to prevent stackoverflow by ofpacts_parse
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12557
Signed-off-by: Yifeng Sun
---
include/openvswitch/ofp-actions.h | 4
lib/ofp-actions.c | 5 +
2 files changed, 9 insertions(+)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index caaa37c05a1d..14c5eab74bd3 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -1175,7 +1175,11 @@ struct ofpact_parse_params {
/* Output. */
struct ofpbuf *ofpacts;
enum ofputil_protocol *usable_protocols;
+
+/* Parse context. */
+unsigned int depth;
};
+#define MAX_OFPACT_PARSE_DEPTH 100
char *ofpacts_parse_actions(const char *, const struct ofpact_parse_params *)
OVS_WARN_UNUSED_RESULT;
char *ofpacts_parse_instructions(const char *,
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index f76db6c0f948..6f175186498d 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -9062,11 +9062,16 @@ static char * OVS_WARN_UNUSED_RESULT
ofpacts_parse(char *str, const struct ofpact_parse_params *pp,
bool allow_instructions, enum ofpact_type outer_action)
{
+if (pp->depth >= MAX_OFPACT_PARSE_DEPTH) {
+return xstrdup("Action nested too deeply");
+}
+CONST_CAST(struct ofpact_parse_params *, pp)->depth++;
uint32_t orig_size = pp->ofpacts->size;
char *error = ofpacts_parse__(str, pp, allow_instructions, outer_action);
if (error) {
pp->ofpacts->size = orig_size;
}
+CONST_CAST(struct ofpact_parse_params *, pp)->depth--;
return error;
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] odp-util: Stop parse odp actions if nlattr is overflow
`encap = nl_msg_start_nested(key, OVS_KEY_ATTR_ENCAP)` ensures that
key->size >= (encap + NLA_HDRLEN), so the `if` statement is safe.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11306
Signed-off-by: Yifeng Sun
---
lib/odp-util.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 778c00ee8876..482a0be2f9d7 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -5599,6 +5599,10 @@ parse_odp_key_mask_attr(struct parse_odp_context
*context, const char *s,
context->depth--;
return retval;
}
+
+if (nl_attr_oversized(key->size - encap - NLA_HDRLEN)) {
+return -E2BIG;
+}
s += retval;
}
s++;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [Nomade Reggae Festival 2019] A 30 minutes de Genève: Les premiers noms, offre Promo
[[HEADLINE]] [[PERMALINK_FULL_LINK]] TITLE OF YOUR EMAIL The content of your email goes here. You can drag and drop blocks of text, images or other content elements to add them to your message. Customize the font and the colors. Add links to track clicks. This is a second paragraph you can customize as your please. If you have stored contact properties with your contacts, you can include personalization variables such as first name, last name in your message content. [[DELIVERY_INFO]] [[POSTAL_ADDRESS]] ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: Failed to merge in the changes. Patch failed at 0001 ofp-packet: Fix NXT_RESUME with geneve tunnel metadata The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces
Oops thank you I don’t know how I looked exactly... :)
On Fri, 1 Feb 2019 at 18:14, Numan Siddique wrote:
>
>
> On Fri, Feb 1, 2019, 7:55 PM Miguel Angel Ajo Pelayo
> wrote:
>
>> Hmm, numan I see an older version of your patch on patchworks [1], but
>> not v6
>> May be there was an issue with mail?
>>
>> [1] https://patchwork.ozlabs.org/patch/973888/
>>
>
> Hi Miguel,
> The patch is merged long back :) -
> https://github.com/openvswitch/ovs/commit/2d661a2733010d7e94560c624edbe7f192952b3d
>
> Thanks
> Numan
>
>
>
>> On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo <
>> majop...@redhat.com> wrote:
>>
>>> Hey,
>>>
>>>Did we get this merged in the end?,
>>>
>>>We're having customers facing issues related to this and we may
>>> need to throttle the BFD settings.y
>>>
>>> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique
>>> wrote:
>>>
On Tue, Oct 9, 2018 at 2:18 PM wrote:
> From: Numan Siddique
>
> With this commit the users can override the default values of
> the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
> This can be useful to debug any issues related to BFD (like
> frequent BFD state changes).
>
> A new column 'options' is added in NB_Global and SB_Global tables
> of OVN_Northbound and OVN_Southbound schemas respectively. CMS
> can define the options 'bfd-min-rx', 'bfd-min-tx',
> 'bfd-decay-min-rx' and 'bfd-mult' in the options column of
> NB_Global table row. ovn-northd copies these options from
> NB_Global to SB_Global. ovn-controller configures these
> options to the tunnel interfaces when enabling BFD.
>
> When BFD is disabled, this patch now clears the 'bfd' column
> of the interface row, instead of setting 'enable_bfd=false'.
>
If this patch is fine, can you please update from 'enable_bfd=false' to
'enable=false' in the last line of the commit message before applying.
Thanks
Numan
>
> Signed-off-by: Numan Siddique
> ---
>
> v5 -> v6
>
> * Addressed the review comments from Ben
> - changed the config options from "bfd:min-rx" to "bfd-min-rx"
> - when disabling the bfd, instead of setting the option
> "enable_bfd=false", now clears the bfd column of the interface
row
>
> v4 -> v5
> ---
> * Addressed a couple of check_patch util warnings - Line exceeding
80
> char
> which was introduced in the v4.
>
> v3 -> v4
>
> * As per the suggestion from Ben - provided the option to
> centrally configuring the BFD params
>
> v2 -> v3
>
> * Added the test case for mult option
>
> v1 -> v2
> ---
> * Addressed review comments by Miguel - added the option 'mult' to
> configure.
>
> ovn/controller/bfd.c| 66
+++--
> ovn/controller/bfd.h| 3 ++
> ovn/controller/ovn-controller.c | 4 +-
> ovn/northd/ovn-northd.c | 2 +
> ovn/ovn-nb.ovsschema| 9 +++--
> ovn/ovn-nb.xml | 35 +
> ovn/ovn-sb.ovsschema| 9 +++--
> ovn/ovn-sb.xml | 38 +++
> tests/ovn.at| 31 +++-
> 9 files changed, 168 insertions(+), 29 deletions(-)
>
> diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> index 051781f38..94dad236e 100644
> --- a/ovn/controller/bfd.c
> +++ b/ovn/controller/bfd.c
> @@ -38,24 +38,6 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
> }
>
> -
> -static void
> -interface_set_bfd(const struct ovsrec_interface *iface, bool
bfd_setting)
> -{
> -const char *new_setting = bfd_setting ? "true":"false";
> -const char *current_setting = smap_get(>bfd, "enable");
> -if (current_setting && !strcmp(current_setting, new_setting)) {
> -/* If already set to the desired setting we skip setting it
again
> - * to avoid flapping to bfd initialization state */
> -return;
> -}
> -const struct smap bfd = SMAP_CONST1(, "enable", new_setting);
> -ovsrec_interface_verify_bfd(iface);
> -ovsrec_interface_set_bfd(iface, );
> -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
> "Disabled",
> -iface->name);
> -}
> -
> void
> bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
> struct sset *active_tunnels)
> @@ -267,6 +249,7 @@ bfd_run(struct ovsdb_idl_index
*sbrec_chassis_by_name,
> const struct
[ovs-dev] [branch 2.9] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
From: Yi-Hung Wei
The patch address vswitchd crash when it receives NXT_RESUME with geneve
tunnel metadata. The crash is due to segmentation fault with the
following stack trace, and it is observed only in kernel datapath.
A test is added to prevent regression.
Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault.
0 0x7fcffd0c5412 in tun_metadata_to_geneve__
(flow=flow@entry=0x7ffcb7106680, b=b@entry=0x7ffcb70eb5a8,
crit_opt=crit_opt@entry=0x7ffcb70eb287)
at lib/tun-metadata.c:676
1 0x7fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow (b=0x7ffcb70eb5a8,
flow=0x7ffcb7106638) at lib/tun-metadata.c:706
2 tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638,
flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry=0x7ffcb70eb5a8)
at lib/tun-metadata.c:810
3 0x7fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8,
tun_key=tun_key@entry=0x7ffcb7106638,
tun_flow_key=tun_flow_key@entry=0x7ffcb7106638,
key_buf=key_buf@entry=0x0, tnl_type=, tnl_type@entry=0x0) at
lib/odp-util.c:2886
4 0x7fcffd0551cf in odp_key_from_dp_packet (buf=buf@entry=0x7ffcb70eb5a8,
packet=0x7ffcb7106590) at lib/odp-util.c:5909
5 0x7fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8,
d_exec=0x7ffcb7106428, dp_ifindex=) at lib/dpif-netlink.c:1873
6 dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00,
ops=ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif-netlink.c:1959
7 0x7fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1,
ops=0x7ffcb7106418, dpif=) at lib/dpif-netlink.c:2258
8 dpif_netlink_operate (dpif_=0xe65e00, ops=, n_ops=) at lib/dpif-netlink.c:2294
9 0x7fcffd014680 in dpif_operate (dpif=, ops=, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif.c:1359
10 0x7fcffd014c58 in dpif_execute (dpif=,
execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324
11 0x7fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50, pin=0x7ffcb7107150) at
ofproto/ofproto-dpif.c:4885
12 0x7fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0,
oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612
13 0x7fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0) at
ofproto/ofproto.c:8137
14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at ofproto/ofproto.c:8258
15 0x7fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0
, ofconn=0xf8c8f0) at ofproto/connmgr.c:1432
16 connmgr_run (mgr=0xe422f0,
handle_openflow=handle_openflow@entry=0x7fcffd4046f0 ) at
ofproto/connmgr.c:363
17 0x7fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821
18 0x0040ca94 in bridge_run__ () at vswitchd/bridge.c:2939
19 0x00411d44 in bridge_run () at vswitchd/bridge.c:2997
20 0x004094fd in main (argc=12, argv=0x7ffcb71085b8) at
vswitchd/ovs-vswitchd.c:119
(cherry-picked from commit bed941ba0f14854683c241fa9bff3d49dd2efeee)
Conflicts:
lib/ofp-packet.c
(branch-2.9 doesn't have lib/ofp-packet.c. The relevant code is in ofp-util.c,
so modified it.)
VMWare-BZ: #2210216
CC: Yi-Hung Wei
Signed-off-by: Numan Siddique
---
lib/ofp-util.c | 2 ++
tests/system-traffic.at | 40
2 files changed, 42 insertions(+)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 0b6f3a3e1..fc5983d37 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3453,6 +3453,7 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool
loose,
error = oxm_decode_match(payload.msg, ofpbuf_msgsize(),
loose, tun_table, vl_mff_map,
>flow_metadata);
+pin->flow_metadata.flow.tunnel.metadata.tab = tun_table;
break;
case NXPINT_USERDATA:
@@ -3618,6 +3619,7 @@ ofputil_decode_packet_in(const struct ofp_header *oh,
bool loose,
enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table,
vl_mff_map, pin, _len,
_id, continuation);
+pin->flow_metadata.flow.tunnel.metadata.tab = tun_table;
if (error) {
return error;
}
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 66d3b1d0f..bd63ad1be 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -380,6 +380,46 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2
10.1.1.100 | FORMAT_PI
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([datapath - flow resume with geneve tun_metadata])
+OVS_CHECK_GENEVE()
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-underlay])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and with a native
+dnl linux device inside the namespace.
Re: [ovs-dev] [PATCH v2] test: Fix failed test "flow resume with geneve tun_metadata"
On Fri, Feb 1, 2019 at 10:02 AM Yifeng Sun wrote:
>
> Test "flow resume with geneve tun_metadata" failed because there is
> no controller running to handle the continuation message. A previous
> commit deleted the line that starts ovs-ofctl as a controller in
> order to avoid a race condition on monitor log. This patch adds
> back this line but omits the log file because this test doesn't
> depend on the log file.
>
> Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition on
> monitor log")
> CC: David Marchand
> Signed-off-by: Yifeng Sun
> ---
Thanks for the fix.
Acked-by: Yi-Hung Wei
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [branch 2.9] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
On Sat, Feb 2, 2019 at 12:28 AM wrote:
> From: Yi-Hung Wei
>
> The patch address vswitchd crash when it receives NXT_RESUME with geneve
> tunnel metadata. The crash is due to segmentation fault with the
> following stack trace, and it is observed only in kernel datapath.
> A test is added to prevent regression.
>
> Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault.
> 0 0x7fcffd0c5412 in tun_metadata_to_geneve__
> (flow=flow@entry=0x7ffcb7106680,
> b=b@entry=0x7ffcb70eb5a8, crit_opt=crit_opt@entry=0x7ffcb70eb287)
>at lib/tun-metadata.c:676
> 1 0x7fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow
> (b=0x7ffcb70eb5a8, flow=0x7ffcb7106638) at lib/tun-metadata.c:706
> 2 tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638,
> flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry
> =0x7ffcb70eb5a8)
>at lib/tun-metadata.c:810
> 3 0x7fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8,
> tun_key=tun_key@entry=0x7ffcb7106638, tun_flow_key=tun_flow_key@entry
> =0x7ffcb7106638,
>key_buf=key_buf@entry=0x0, tnl_type=, tnl_type@entry=0x0)
> at lib/odp-util.c:2886
> 4 0x7fcffd0551cf in odp_key_from_dp_packet (buf=buf@entry=0x7ffcb70eb5a8,
> packet=0x7ffcb7106590) at lib/odp-util.c:5909
> 5 0x7fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8,
> d_exec=0x7ffcb7106428, dp_ifindex=) at
> lib/dpif-netlink.c:1873
> 6 dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00,
> ops=ops@entry=0x7ffcb7106418,
> n_ops=n_ops@entry=1) at lib/dpif-netlink.c:1959
> 7 0x7fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1,
> ops=0x7ffcb7106418, dpif=) at lib/dpif-netlink.c:2258
> 8 dpif_netlink_operate (dpif_=0xe65e00, ops=,
> n_ops=) at lib/dpif-netlink.c:2294
> 9 0x7fcffd014680 in dpif_operate (dpif=,
> ops=, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at
> lib/dpif.c:1359
> 10 0x7fcffd014c58 in dpif_execute (dpif=,
> execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324
> 11 0x7fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50,
> pin=0x7ffcb7107150) at ofproto/ofproto-dpif.c:4885
> 12 0x7fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0,
> oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612
> 13 0x7fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0)
> at ofproto/ofproto.c:8137
> 14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at
> ofproto/ofproto.c:8258
> 15 0x7fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0
> , ofconn=0xf8c8f0) at ofproto/connmgr.c:1432
> 16 connmgr_run (mgr=0xe422f0,
> handle_openflow=handle_openflow@entry=0x7fcffd4046f0
> ) at ofproto/connmgr.c:363
> 17 0x7fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821
> 18 0x0040ca94 in bridge_run__ () at vswitchd/bridge.c:2939
> 19 0x00411d44 in bridge_run () at vswitchd/bridge.c:2997
> 20 0x004094fd in main (argc=12, argv=0x7ffcb71085b8) at
> vswitchd/ovs-vswitchd.c:119
>
> (cherry-picked from commit bed941ba0f14854683c241fa9bff3d49dd2efeee)
> Conflicts:
> lib/ofp-packet.c
> (branch-2.9 doesn't have lib/ofp-packet.c. The relevant code is in
> ofp-util.c, so modified it.)
>
>
Hi Yi-Hung We,
I have backported this patch to branch 2.9. Could you please provide your
Signed-off-by tag if the fix is fine.
Thanks
Numan
> VMWare-BZ: #2210216
> CC: Yi-Hung Wei
> Signed-off-by: Numan Siddique
> ---
> lib/ofp-util.c | 2 ++
> tests/system-traffic.at | 40
> 2 files changed, 42 insertions(+)
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 0b6f3a3e1..fc5983d37 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -3453,6 +3453,7 @@ decode_nx_packet_in2(const struct ofp_header *oh,
> bool loose,
> error = oxm_decode_match(payload.msg,
> ofpbuf_msgsize(),
> loose, tun_table, vl_mff_map,
> >flow_metadata);
> +pin->flow_metadata.flow.tunnel.metadata.tab = tun_table;
> break;
>
> case NXPINT_USERDATA:
> @@ -3618,6 +3619,7 @@ ofputil_decode_packet_in(const struct ofp_header
> *oh, bool loose,
> enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table,
> vl_mff_map, pin,
> _len,
> _id,
> continuation);
> +pin->flow_metadata.flow.tunnel.metadata.tab = tun_table;
> if (error) {
> return error;
> }
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 66d3b1d0f..bd63ad1be 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -380,6 +380,46 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3
> -w 2 10.1.1.100 | FORMAT_PI
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([datapath - flow resume with geneve tun_metadata])
> +OVS_CHECK_GENEVE()
> +
> +OVS_TRAFFIC_VSWITCHD_START()
>
Re: [ovs-dev] [branch 2.9] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
On Fri, Feb 1, 2019 at 10:58 AM wrote: > > From: Yi-Hung Wei > > The patch address vswitchd crash when it receives NXT_RESUME with geneve > tunnel metadata. The crash is due to segmentation fault with the > following stack trace, and it is observed only in kernel datapath. > A test is added to prevent regression. > > Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault. > 0 0x7fcffd0c5412 in tun_metadata_to_geneve__ > (flow=flow@entry=0x7ffcb7106680, b=b@entry=0x7ffcb70eb5a8, > crit_opt=crit_opt@entry=0x7ffcb70eb287) >at lib/tun-metadata.c:676 > 1 0x7fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow > (b=0x7ffcb70eb5a8, flow=0x7ffcb7106638) at lib/tun-metadata.c:706 > 2 tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638, > flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry=0x7ffcb70eb5a8) >at lib/tun-metadata.c:810 > 3 0x7fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8, > tun_key=tun_key@entry=0x7ffcb7106638, > tun_flow_key=tun_flow_key@entry=0x7ffcb7106638, >key_buf=key_buf@entry=0x0, tnl_type=, tnl_type@entry=0x0) > at lib/odp-util.c:2886 > 4 0x7fcffd0551cf in odp_key_from_dp_packet > (buf=buf@entry=0x7ffcb70eb5a8, packet=0x7ffcb7106590) at lib/odp-util.c:5909 > 5 0x7fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8, > d_exec=0x7ffcb7106428, dp_ifindex=) at lib/dpif-netlink.c:1873 > 6 dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00, > ops=ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif-netlink.c:1959 > 7 0x7fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1, > ops=0x7ffcb7106418, dpif=) at lib/dpif-netlink.c:2258 > 8 dpif_netlink_operate (dpif_=0xe65e00, ops=, > n_ops=) at lib/dpif-netlink.c:2294 > 9 0x7fcffd014680 in dpif_operate (dpif=, ops= out>, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif.c:1359 > 10 0x7fcffd014c58 in dpif_execute (dpif=, > execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324 > 11 0x7fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50, pin=0x7ffcb7107150) > at ofproto/ofproto-dpif.c:4885 > 12 0x7fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0, > oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612 > 13 0x7fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0) at > ofproto/ofproto.c:8137 > 14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at > ofproto/ofproto.c:8258 > 15 0x7fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0 > , ofconn=0xf8c8f0) at ofproto/connmgr.c:1432 > 16 connmgr_run (mgr=0xe422f0, > handle_openflow=handle_openflow@entry=0x7fcffd4046f0 ) at > ofproto/connmgr.c:363 > 17 0x7fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821 > 18 0x0040ca94 in bridge_run__ () at vswitchd/bridge.c:2939 > 19 0x00411d44 in bridge_run () at vswitchd/bridge.c:2997 > 20 0x004094fd in main (argc=12, argv=0x7ffcb71085b8) at > vswitchd/ovs-vswitchd.c:119 > > (cherry-picked from commit bed941ba0f14854683c241fa9bff3d49dd2efeee) > Conflicts: > lib/ofp-packet.c > (branch-2.9 doesn't have lib/ofp-packet.c. The relevant code is in > ofp-util.c, so modified it.) > > VMWare-BZ: #2210216 > CC: Yi-Hung Wei > Signed-off-by: Numan Siddique Thanks for the backport. Signed-off-by: Yi-Hung Wei ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-dpdk-macros.at: Drop dpdk-socket-mem configuration.
Ilya Maximets writes: > There are two reasons: > 1. OVS provides same default itself. > 2. socket-mem is not necessary with dynamic memory model in DPDK 18.11. > > Signed-off-by: Ilya Maximets > --- This should probably have been addressed with the update to 18.11. Thanks for it, Ilya! Acked-by: Aaron Conole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] datapath: Avoid OOB read when parsing flow nlattrs
On 2/1/2019 9:59 AM, Aaron Conole wrote: Gregory Rose writes: Hmmm... I used the accepted (or at least previously accepted) method for upstream commits. Has something changed? Nope - the robot usually bleeps about these backports (historically, it seems to anyway). I'll try to set aside some time next week and look into it a bit. Ah, I see. Well, in the realm of things we need to work on it seems lower priority but thanks for the explanation. That's good enough for me. Regards, - Greg - Greg On 1/31/2019 1:59 PM, 0-day Robot wrote: Bleep bloop. Greetings Greg Rose, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author Ross Lagerwall needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Greg Rose Lines checked: 46, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] datapath: Avoid OOB read when parsing flow nlattrs
Gregory Rose writes: > Hmmm... > > I used the accepted (or at least previously accepted) method for > upstream commits. Has something changed? Nope - the robot usually bleeps about these backports (historically, it seems to anyway). I'll try to set aside some time next week and look into it a bit. > - Greg > > On 1/31/2019 1:59 PM, 0-day Robot wrote: >> Bleep bloop. Greetings Greg Rose, I am a robot and I have tried out your >> patch. >> Thanks for your contribution. >> >> I encountered some error that I wasn't expecting. See the details below. >> >> >> checkpatch: >> ERROR: Author Ross Lagerwall needs to sign off. >> WARNING: Unexpected sign-offs from developers who are not authors or >> co-authors or committers: Greg Rose >> Lines checked: 46, Warnings: 1, Errors: 1 >> >> >> Please check this out. If you feel there has been an error, please email >> acon...@bytheb.org >> >> Thanks, >> 0-day Robot > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] test: Fix failed test "flow resume with geneve tun_metadata"
Test "flow resume with geneve tun_metadata" failed because there is
no controller running to handle the continuation message. A previous
commit deleted the line that starts ovs-ofctl as a controller in
order to avoid a race condition on monitor log. This patch adds
back this line but omits the log file because this test doesn't
depend on the log file.
Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition on
monitor log")
CC: David Marchand
Signed-off-by: Yifeng Sun
---
v1->v2: Fixed commit message by Darrell's suggestion, thanks Darrell!
tests/system-traffic.at | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index ffe508dd61f7..84c2af4170a3 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -538,6 +538,8 @@ OVS_CHECK_GENEVE()
OVS_TRAFFIC_VSWITCHD_START()
ADD_BR([br-underlay])
+AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2>
/dev/null])
+
ADD_NAMESPACES(at_ns0)
dnl Set up underlay link from host into the namespace using veth pair.
@@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 |
FORMAT_PING], [0], [dnl
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])
+OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces
On Fri, Feb 1, 2019, 7:55 PM Miguel Angel Ajo Pelayo
wrote:
> Hmm, numan I see an older version of your patch on patchworks [1], but not
> v6
> May be there was an issue with mail?
>
> [1] https://patchwork.ozlabs.org/patch/973888/
>
Hi Miguel,
The patch is merged long back :) -
https://github.com/openvswitch/ovs/commit/2d661a2733010d7e94560c624edbe7f192952b3d
Thanks
Numan
> On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo <
> majop...@redhat.com> wrote:
>
>> Hey,
>>
>>Did we get this merged in the end?,
>>
>>We're having customers facing issues related to this and we may
>> need to throttle the BFD settings.y
>>
>> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique
>> wrote:
>>
>>> On Tue, Oct 9, 2018 at 2:18 PM wrote:
>>>
>>> > From: Numan Siddique
>>> >
>>> > With this commit the users can override the default values of
>>> > the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
>>> > This can be useful to debug any issues related to BFD (like
>>> > frequent BFD state changes).
>>> >
>>> > A new column 'options' is added in NB_Global and SB_Global tables
>>> > of OVN_Northbound and OVN_Southbound schemas respectively. CMS
>>> > can define the options 'bfd-min-rx', 'bfd-min-tx',
>>> > 'bfd-decay-min-rx' and 'bfd-mult' in the options column of
>>> > NB_Global table row. ovn-northd copies these options from
>>> > NB_Global to SB_Global. ovn-controller configures these
>>> > options to the tunnel interfaces when enabling BFD.
>>> >
>>> > When BFD is disabled, this patch now clears the 'bfd' column
>>> > of the interface row, instead of setting 'enable_bfd=false'.
>>> >
>>>
>>> If this patch is fine, can you please update from 'enable_bfd=false' to
>>> 'enable=false' in the last line of the commit message before applying.
>>>
>>> Thanks
>>> Numan
>>>
>>>
>>> >
>>> > Signed-off-by: Numan Siddique
>>> > ---
>>> >
>>> > v5 -> v6
>>> >
>>> > * Addressed the review comments from Ben
>>> > - changed the config options from "bfd:min-rx" to "bfd-min-rx"
>>> > - when disabling the bfd, instead of setting the option
>>> > "enable_bfd=false", now clears the bfd column of the interface
>>> row
>>> >
>>> > v4 -> v5
>>> > ---
>>> > * Addressed a couple of check_patch util warnings - Line exceeding 80
>>> > char
>>> > which was introduced in the v4.
>>> >
>>> > v3 -> v4
>>> >
>>> > * As per the suggestion from Ben - provided the option to
>>> > centrally configuring the BFD params
>>> >
>>> > v2 -> v3
>>> >
>>> > * Added the test case for mult option
>>> >
>>> > v1 -> v2
>>> > ---
>>> > * Addressed review comments by Miguel - added the option 'mult' to
>>> > configure.
>>> >
>>> > ovn/controller/bfd.c| 66 +++--
>>> > ovn/controller/bfd.h| 3 ++
>>> > ovn/controller/ovn-controller.c | 4 +-
>>> > ovn/northd/ovn-northd.c | 2 +
>>> > ovn/ovn-nb.ovsschema| 9 +++--
>>> > ovn/ovn-nb.xml | 35 +
>>> > ovn/ovn-sb.ovsschema| 9 +++--
>>> > ovn/ovn-sb.xml | 38 +++
>>> > tests/ovn.at| 31 +++-
>>> > 9 files changed, 168 insertions(+), 29 deletions(-)
>>> >
>>> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
>>> > index 051781f38..94dad236e 100644
>>> > --- a/ovn/controller/bfd.c
>>> > +++ b/ovn/controller/bfd.c
>>> > @@ -38,24 +38,6 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>>> > ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
>>> > }
>>> >
>>> > -
>>> > -static void
>>> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
>>> bfd_setting)
>>> > -{
>>> > -const char *new_setting = bfd_setting ? "true":"false";
>>> > -const char *current_setting = smap_get(>bfd, "enable");
>>> > -if (current_setting && !strcmp(current_setting, new_setting)) {
>>> > -/* If already set to the desired setting we skip setting it
>>> again
>>> > - * to avoid flapping to bfd initialization state */
>>> > -return;
>>> > -}
>>> > -const struct smap bfd = SMAP_CONST1(, "enable", new_setting);
>>> > -ovsrec_interface_verify_bfd(iface);
>>> > -ovsrec_interface_set_bfd(iface, );
>>> > -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
>>> > "Disabled",
>>> > -iface->name);
>>> > -}
>>> > -
>>> > void
>>> > bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
>>> > struct sset *active_tunnels)
>>> > @@ -267,6 +249,7 @@ bfd_run(struct ovsdb_idl_index
>>> *sbrec_chassis_by_name,
>>> > const struct ovsrec_interface_table *interface_table,
>>> > const struct ovsrec_bridge *br_int,
>>> > const struct sbrec_chassis *chassis_rec,
>>> > +const struct sbrec_sb_global_table *sb_global_table,
>>> > const struct hmap *local_datapaths)
>>> > {
>>> >
[ovs-dev] [RFC v1] lib/tc: add ingress ratelimiting support for tc-offload
Firstly this patch introduces the notion of reserved priority, as the
filter implementing ingress policing would require the highest priority.
Secondly it allows setting rate limiters while tc-offloads has been
enabled. Lastly it installs a matchall filter that matches all traffic
and then applies a police action, when configuring an ingress rate
limiter.
An example of what to expect:
OvS CLI:
ovs-vsctl set interface ingress_policing_rate=5000
ovs-vsctl set interface ingress_policing_burst=100
Resulting TC filter:
filter protocol ip pref 1 matchall chain 0
filter protocol ip pref 1 matchall chain 0 handle 0x1
not_in_hw
action order 1: police 0x1 rate 5Mbit burst 125Kb mtu 64Kb
action drop/continue overhead 0b
ref 1 bind 1 installed 3 sec used 3 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
10.0.0.200 () port 0 AF_INET : demo
Recv SendSend
Socket Socket Message Elapsed
Size SizeSize Time Throughput
bytes bytes bytessecs.10^6bits/sec
131072 16384 1638460.13 4.49
ovs-vsctl list interface
_uuid : 2ca774e8-8b95-430f-a2c2-f8f742613ab1
admin_state : up
...
ingress_policing_burst: 100
ingress_policing_rate: 5000
...
type: ""
Signed-off-by: Pieter Jansen van Vuuren
Reviewed-by: Simon Horman
---
include/linux/pkt_cls.h | 12
lib/netdev-linux.c | 131 +++
lib/netdev-tc-offloads.c | 2 +-
lib/tc.c | 4 ++
lib/tc.h | 7 +++
5 files changed, 144 insertions(+), 12 deletions(-)
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 1384d71f9..4adea59e7 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -238,6 +238,18 @@ enum {
TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
};
+/* Match-all classifier */
+
+enum {
+ TCA_MATCHALL_UNSPEC,
+ TCA_MATCHALL_CLASSID,
+ TCA_MATCHALL_ACT,
+ TCA_MATCHALL_FLAGS,
+ __TCA_MATCHALL_MAX,
+};
+
+#define TCA_MATCHALL_MAX (__TCA_MATCHALL_MAX - 1)
+
#endif /* __KERNEL__ || !HAVE_TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST */
#endif /* __LINUX_PKT_CLS_WRAPPER_H */
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 25d037cb6..92cfb229d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -113,6 +113,10 @@ COVERAGE_DEFINE(netdev_set_ethtool);
#define TC_RTAB_SIZE 1024
#endif
+#ifndef TCM_IFINDEX_MAGIC_BLOCK
+#define TCM_IFINDEX_MAGIC_BLOCK (0xU)
+#endif
+
/* Linux 2.6.21 introduced struct tpacket_auxdata.
* Linux 2.6.27 added the tp_vlan_tci member.
* Linux 3.0 defined TP_STATUS_VLAN_VALID.
@@ -473,10 +477,10 @@ static int tc_delete_class(const struct netdev *,
unsigned int handle);
static int tc_del_qdisc(struct netdev *netdev);
static int tc_query_qdisc(const struct netdev *netdev);
+void
+tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate);
static int tc_calc_cell_log(unsigned int mtu);
static void tc_fill_rate(struct tc_ratespec *rate, uint64_t bps, int mtu);
-static void tc_put_rtab(struct ofpbuf *, uint16_t type,
-const struct tc_ratespec *rate);
static int tc_calc_buffer(unsigned int Bps, int mtu, uint64_t burst_bytes);
struct netdev_linux {
@@ -2324,6 +2328,109 @@ exit:
return error;
}
+static struct tc_police
+tc_matchall_fill_police(uint32_t kbits_rate, uint32_t kbits_burst)
+{
+unsigned int bsize = MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 64;
+unsigned int bps = ((uint64_t) kbits_rate * 1000) / 8;
+struct tc_police police;
+struct tc_ratespec rate;
+int mtu = 65535;
+
+memset(, 0, sizeof rate);
+rate.rate = bps;
+rate.cell_log = tc_calc_cell_log(mtu);
+rate.mpu = ETH_TOTAL_MIN;
+
+memset(, 0, sizeof police);
+police.burst = tc_bytes_to_ticks(bps, bsize);
+police.action = TC_POLICE_SHOT;
+police.rate = rate;
+police.mtu = mtu;
+
+return police;
+}
+
+static void
+nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police)
+{
+size_t offset;
+
+nl_msg_put_string(request, TCA_ACT_KIND, "police");
+offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
+nl_msg_put_unspec(request, TCA_POLICE_TBF, , sizeof police);
+tc_put_rtab(request, TCA_POLICE_RATE, );
+nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_UNSPEC);
+nl_msg_end_nested(request, offset);
+}
+
+static int
+tc_add_matchall_policer(struct netdev *netdev, uint32_t kbits_rate,
+uint32_t kbits_burst)
+{
+uint16_t eth_type = (OVS_FORCE uint16_t) htons(ETH_P_ALL);
+size_t basic_offset, action_offset, inner_offset;
+uint16_t prio = TC_RESERVED_PRIORITY_POLICE;
+int ifindex, index, err = 0;
+struct tc_police pol_act;
+uint32_t block_id = 0;
+struct ofpbuf request;
+
Re: [ovs-dev] [PATCH v2] ovs-tcpdump: Fix an undefined variable
Ilya Maximets writes:
>> Run ovs-tcpdump without --span, and it throws the following
>> exception. Define mirror_select_all to avoid the error.
>>
>> Traceback (most recent call last):
>> File "/usr/local/bin/ovs-tcpdump", line 488, in
>> main()
>> File "/usr/local/bin/ovs-tcpdump", line 454, in main
>> mirror_select_all)
>> UnboundLocalError: local variable 'mirror_select_all' referenced before
>> assignment
>>
>> Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on
>> bridge.")
>>
>> Signed-off-by: Hyong Youb Kim
>> ---
>> v2:
>> * fix a typo: with -> without
>> * resend after subscribing to dev to avoid ovs-dev in From:
>>
>> utilities/ovs-tcpdump.in | 1 +
>> 1 file changed, 1 insertion(+)
>
> Thanks for the fix,
> Acked-by: Ilya Maximets
>
> Regarding checkpatch. Looks like it's a patchwork issue.
> It has mail-list email in a form field. Mail-list itself shows correct e-mail.
> I hope, maintainers will use correct one while applying the patch, if needed.
I don't see that the email address is correct. Actually in my email
client, it does say:
From: Hyong Youb Kim via dev
I'm not sure what causes this (again, I'm going from the email I
received), but we've seen it before.
> Another option (workaround): You may add "From: Name " as a first line
> of the commit-message. This should be correctly treated by git in any case.
Also, you can try attaching the patch to an email to the list (I think
that can work), or submitting a pull request on github and sending the
list an email with the information.
Very strange, though.
>> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
>> index 22f249f58..269c252f8 100755
>> --- a/utilities/ovs-tcpdump.in
>> +++ b/utilities/ovs-tcpdump.in
>> @@ -379,6 +379,7 @@ def main():
>>
>> skip_next = False
>> mirror_interface = None
>> +mirror_select_all = False
>> dump_cmd = 'tcpdump'
>>
>> for cur, nxt in argv_tuples(sys.argv[1:]):
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovs-tcpdump: Fix an undefined variable
> Run ovs-tcpdump without --span, and it throws the following
> exception. Define mirror_select_all to avoid the error.
>
> Traceback (most recent call last):
> File "/usr/local/bin/ovs-tcpdump", line 488, in
> main()
> File "/usr/local/bin/ovs-tcpdump", line 454, in main
> mirror_select_all)
> UnboundLocalError: local variable 'mirror_select_all' referenced before
> assignment
>
> Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.")
>
> Signed-off-by: Hyong Youb Kim
> ---
> v2:
> * fix a typo: with -> without
> * resend after subscribing to dev to avoid ovs-dev in From:
>
> utilities/ovs-tcpdump.in | 1 +
> 1 file changed, 1 insertion(+)
Thanks for the fix,
Acked-by: Ilya Maximets
Regarding checkpatch. Looks like it's a patchwork issue.
It has mail-list email in a form field. Mail-list itself shows correct e-mail.
I hope, maintainers will use correct one while applying the patch, if needed.
Another option (workaround): You may add "From: Name " as a first line
of the commit-message. This should be correctly treated by git in any case.
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 22f249f58..269c252f8 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -379,6 +379,7 @@ def main():
>
> skip_next = False
> mirror_interface = None
> +mirror_select_all = False
> dump_cmd = 'tcpdump'
>
> for cur, nxt in argv_tuples(sys.argv[1:]):
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Remove unnecessary check in process_ftp_ctl_v4
On Fri, Feb 1, 2019 at 1:07 AM Li,Rongqing wrote: > > > 发件人: Darrell Ball [mailto:dlu...@gmail.com] > 发送时间: 2019年2月1日 16:15 > 收件人: Li,Rongqing > 抄送: ovs dev > 主题: Re: [ovs-dev] [PATCH] conntrack: Remove unnecessary check in > process_ftp_ctl_v4 > > > >This was intentionally done to be documentative and also make it hard to > break; > >this code path sees a tiny number of packets. > >I am not sure there is much to gain by removing it and adding in lieu of > comments ? > > > gain for packets is little, but step by step. > > And it can reduce unnessesary codes, make a newbie to easy study > I agree; the useless range check can also be considered misleading, in retrospect. Dropping port_lo_hs should not be that confusing. Can you resend the patch with the missing 'Co-authored-by' tag. Co-authored-by: Wang Li > Thanks > > -RongQing > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: compute hash value once when calling conn_key_lookup
On Fri, Feb 1, 2019 at 12:49 AM Li,Rongqing wrote: > > > 发件人: Darrell Ball [mailto:dlu...@gmail.com] > 发送时间: 2019年2月1日 16:25 > 收件人: Li,Rongqing > 抄送: ovs dev > 主题: Re: [ovs-dev] [PATCH] conntrack: compute hash value once when calling > conn_key_lookup > > > The function comment says it all: > > /* Typical usage of this helper is in non per-packet code; > > * this is because the bucket lock needs to be held for lookup > > * and a hash would have already been needed. Hence, this function > > * is just intended for code clarity. */ > > > > Does it mean that the hash value will be changed ? No > if it is true, should we release original lock, and relock lock for new > hash value? > Also, the full context is this: The function is also written with the intention to remove all the 'bucket' stuff in a future patchset (check the ML). It turns out, the function will get deprecated in future. > > Thanks > > -RongQing > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] system-dpdk-macros.at: Drop dpdk-socket-mem configuration.
There are two reasons:
1. OVS provides same default itself.
2. socket-mem is not necessary with dynamic memory model in DPDK 18.11.
Signed-off-by: Ilya Maximets
---
tests/system-dpdk-macros.at | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index f772a1945..c6708caaf 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -57,9 +57,6 @@ m4_define([OVS_DPDK_START],
dnl Enable DPDK functionality
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
other_config:dpdk-init=true])
- AT_CHECK([lscpu], [], [stdout])
- AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3))
{printf "1024,"}; print "1024"}' > SOCKET_MEM])
- AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
other_config:dpdk-socket-mem="$(cat SOCKET_MEM)"])
dnl Start ovs-vswitchd.
AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn
-vofproto_dpif -vunixctl], [0], [stdout], [stderr])
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces
Hmm, numan I see an older version of your patch on patchworks [1], but not
v6
May be there was an issue with mail?
[1] https://patchwork.ozlabs.org/patch/973888/
On Fri, Feb 1, 2019 at 3:17 PM Miguel Angel Ajo Pelayo
wrote:
> Hey,
>
>Did we get this merged in the end?,
>
>We're having customers facing issues related to this and we may
> need to throttle the BFD settings.y
>
> On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique
> wrote:
>
>> On Tue, Oct 9, 2018 at 2:18 PM wrote:
>>
>> > From: Numan Siddique
>> >
>> > With this commit the users can override the default values of
>> > the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
>> > This can be useful to debug any issues related to BFD (like
>> > frequent BFD state changes).
>> >
>> > A new column 'options' is added in NB_Global and SB_Global tables
>> > of OVN_Northbound and OVN_Southbound schemas respectively. CMS
>> > can define the options 'bfd-min-rx', 'bfd-min-tx',
>> > 'bfd-decay-min-rx' and 'bfd-mult' in the options column of
>> > NB_Global table row. ovn-northd copies these options from
>> > NB_Global to SB_Global. ovn-controller configures these
>> > options to the tunnel interfaces when enabling BFD.
>> >
>> > When BFD is disabled, this patch now clears the 'bfd' column
>> > of the interface row, instead of setting 'enable_bfd=false'.
>> >
>>
>> If this patch is fine, can you please update from 'enable_bfd=false' to
>> 'enable=false' in the last line of the commit message before applying.
>>
>> Thanks
>> Numan
>>
>>
>> >
>> > Signed-off-by: Numan Siddique
>> > ---
>> >
>> > v5 -> v6
>> >
>> > * Addressed the review comments from Ben
>> > - changed the config options from "bfd:min-rx" to "bfd-min-rx"
>> > - when disabling the bfd, instead of setting the option
>> > "enable_bfd=false", now clears the bfd column of the interface row
>> >
>> > v4 -> v5
>> > ---
>> > * Addressed a couple of check_patch util warnings - Line exceeding 80
>> > char
>> > which was introduced in the v4.
>> >
>> > v3 -> v4
>> >
>> > * As per the suggestion from Ben - provided the option to
>> > centrally configuring the BFD params
>> >
>> > v2 -> v3
>> >
>> > * Added the test case for mult option
>> >
>> > v1 -> v2
>> > ---
>> > * Addressed review comments by Miguel - added the option 'mult' to
>> > configure.
>> >
>> > ovn/controller/bfd.c| 66 +++--
>> > ovn/controller/bfd.h| 3 ++
>> > ovn/controller/ovn-controller.c | 4 +-
>> > ovn/northd/ovn-northd.c | 2 +
>> > ovn/ovn-nb.ovsschema| 9 +++--
>> > ovn/ovn-nb.xml | 35 +
>> > ovn/ovn-sb.ovsschema| 9 +++--
>> > ovn/ovn-sb.xml | 38 +++
>> > tests/ovn.at| 31 +++-
>> > 9 files changed, 168 insertions(+), 29 deletions(-)
>> >
>> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
>> > index 051781f38..94dad236e 100644
>> > --- a/ovn/controller/bfd.c
>> > +++ b/ovn/controller/bfd.c
>> > @@ -38,24 +38,6 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>> > ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
>> > }
>> >
>> > -
>> > -static void
>> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
>> bfd_setting)
>> > -{
>> > -const char *new_setting = bfd_setting ? "true":"false";
>> > -const char *current_setting = smap_get(>bfd, "enable");
>> > -if (current_setting && !strcmp(current_setting, new_setting)) {
>> > -/* If already set to the desired setting we skip setting it
>> again
>> > - * to avoid flapping to bfd initialization state */
>> > -return;
>> > -}
>> > -const struct smap bfd = SMAP_CONST1(, "enable", new_setting);
>> > -ovsrec_interface_verify_bfd(iface);
>> > -ovsrec_interface_set_bfd(iface, );
>> > -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
>> > "Disabled",
>> > -iface->name);
>> > -}
>> > -
>> > void
>> > bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
>> > struct sset *active_tunnels)
>> > @@ -267,6 +249,7 @@ bfd_run(struct ovsdb_idl_index
>> *sbrec_chassis_by_name,
>> > const struct ovsrec_interface_table *interface_table,
>> > const struct ovsrec_bridge *br_int,
>> > const struct sbrec_chassis *chassis_rec,
>> > +const struct sbrec_sb_global_table *sb_global_table,
>> > const struct hmap *local_datapaths)
>> > {
>> >
>> > @@ -292,15 +275,58 @@ bfd_run(struct ovsdb_idl_index
>> > *sbrec_chassis_by_name,
>> > }
>> > }
>> >
>> > +const struct sbrec_sb_global *sb
>> > += sbrec_sb_global_table_first(sb_global_table);
>> > +struct smap bfd = SMAP_INITIALIZER();
>> > +smap_add(, "enable", "true");
>> > +
>> > +if (sb) {
>> > +const char
Re: [ovs-dev] [PATCH v6] ovn: Support configuring the BFD params for the tunnel interfaces
Hey,
Did we get this merged in the end?,
We're having customers facing issues related to this and we may
need to throttle the BFD settings.y
On Tue, Oct 9, 2018 at 10:54 AM Numan Siddique wrote:
> On Tue, Oct 9, 2018 at 2:18 PM wrote:
>
> > From: Numan Siddique
> >
> > With this commit the users can override the default values of
> > the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
> > This can be useful to debug any issues related to BFD (like
> > frequent BFD state changes).
> >
> > A new column 'options' is added in NB_Global and SB_Global tables
> > of OVN_Northbound and OVN_Southbound schemas respectively. CMS
> > can define the options 'bfd-min-rx', 'bfd-min-tx',
> > 'bfd-decay-min-rx' and 'bfd-mult' in the options column of
> > NB_Global table row. ovn-northd copies these options from
> > NB_Global to SB_Global. ovn-controller configures these
> > options to the tunnel interfaces when enabling BFD.
> >
> > When BFD is disabled, this patch now clears the 'bfd' column
> > of the interface row, instead of setting 'enable_bfd=false'.
> >
>
> If this patch is fine, can you please update from 'enable_bfd=false' to
> 'enable=false' in the last line of the commit message before applying.
>
> Thanks
> Numan
>
>
> >
> > Signed-off-by: Numan Siddique
> > ---
> >
> > v5 -> v6
> >
> > * Addressed the review comments from Ben
> > - changed the config options from "bfd:min-rx" to "bfd-min-rx"
> > - when disabling the bfd, instead of setting the option
> > "enable_bfd=false", now clears the bfd column of the interface row
> >
> > v4 -> v5
> > ---
> > * Addressed a couple of check_patch util warnings - Line exceeding 80
> > char
> > which was introduced in the v4.
> >
> > v3 -> v4
> >
> > * As per the suggestion from Ben - provided the option to
> > centrally configuring the BFD params
> >
> > v2 -> v3
> >
> > * Added the test case for mult option
> >
> > v1 -> v2
> > ---
> > * Addressed review comments by Miguel - added the option 'mult' to
> > configure.
> >
> > ovn/controller/bfd.c| 66 +++--
> > ovn/controller/bfd.h| 3 ++
> > ovn/controller/ovn-controller.c | 4 +-
> > ovn/northd/ovn-northd.c | 2 +
> > ovn/ovn-nb.ovsschema| 9 +++--
> > ovn/ovn-nb.xml | 35 +
> > ovn/ovn-sb.ovsschema| 9 +++--
> > ovn/ovn-sb.xml | 38 +++
> > tests/ovn.at| 31 +++-
> > 9 files changed, 168 insertions(+), 29 deletions(-)
> >
> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> > index 051781f38..94dad236e 100644
> > --- a/ovn/controller/bfd.c
> > +++ b/ovn/controller/bfd.c
> > @@ -38,24 +38,6 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
> > }
> >
> > -
> > -static void
> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
> bfd_setting)
> > -{
> > -const char *new_setting = bfd_setting ? "true":"false";
> > -const char *current_setting = smap_get(>bfd, "enable");
> > -if (current_setting && !strcmp(current_setting, new_setting)) {
> > -/* If already set to the desired setting we skip setting it
> again
> > - * to avoid flapping to bfd initialization state */
> > -return;
> > -}
> > -const struct smap bfd = SMAP_CONST1(, "enable", new_setting);
> > -ovsrec_interface_verify_bfd(iface);
> > -ovsrec_interface_set_bfd(iface, );
> > -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
> > "Disabled",
> > -iface->name);
> > -}
> > -
> > void
> > bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
> > struct sset *active_tunnels)
> > @@ -267,6 +249,7 @@ bfd_run(struct ovsdb_idl_index
> *sbrec_chassis_by_name,
> > const struct ovsrec_interface_table *interface_table,
> > const struct ovsrec_bridge *br_int,
> > const struct sbrec_chassis *chassis_rec,
> > +const struct sbrec_sb_global_table *sb_global_table,
> > const struct hmap *local_datapaths)
> > {
> >
> > @@ -292,15 +275,58 @@ bfd_run(struct ovsdb_idl_index
> > *sbrec_chassis_by_name,
> > }
> > }
> >
> > +const struct sbrec_sb_global *sb
> > += sbrec_sb_global_table_first(sb_global_table);
> > +struct smap bfd = SMAP_INITIALIZER();
> > +smap_add(, "enable", "true");
> > +
> > +if (sb) {
> > +const char *min_rx = smap_get(>options, "bfd-min-rx");
> > +const char *decay_min_rx = smap_get(>options,
> > "bfd-decay-min-rx");
> > +const char *min_tx = smap_get(>options, "bfd-min-tx");
> > +const char *mult = smap_get(>options, "bfd-mult");
> > +if (min_rx) {
> > +smap_add(, "min_rx", min_rx);
> > +}
> > +if
Re: [ovs-dev] [PATCH v2 2/2] dpdk: Limit DPDK memory usage.
On 01.02.2019 16:51, Ian Stokes wrote:
> On 1/31/2019 2:17 PM, Ilya Maximets wrote:
>> On 31.01.2019 15:39, Ian Stokes wrote:
>>> On 1/29/2019 8:11 AM, Ilya Maximets wrote:
Since 18.05 release, DPDK moved to dynamic memory model in which
hugepages could be allocated on demand. At the same time '--socket-mem'
option was re-defined as a size of pre-allocated memory, i.e. memory
that should be allocated at startup and could not be freed.
So, DPDK with a new memory model could allocate more hugepage memory
than specified in '--socket-mem' or '-m' options.
This change adds new configurable 'other_config:dpdk-socket-limit'
which could be used to limit the ammount of memory DPDK could use.
It uses new DPDK option '--socket-limit'.
Ex.:
ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-limit="1024,1024"
Also, in order to preserve old behaviour, if '--socket-limit' is not
specified, it will be defaulted to the amount of memory specified by
'--socket-mem' option, i.e. OVS will not be able to allocate more.
This is needed, for example, to disallow OVS to allocate more memory
than reserved for it by Nova in OpenStack installations.
Signed-off-by: Ilya Maximets
>>>
>>> Thanks for this Ilya. Given that this will affect 2.11 as well, it probably
>>> makes sense to back port it (as well as the svec patch in this series) to
>>> OVS 2.11 so as to ensure uniform memory behavior between 2.10 to 2.11.
>>
>> Yes. This probably makes sense.
>
> Great, I've pushed to master and backported to 2.11.
Thanks.
>>
>>>
>>> One comment below also.
>>>
---
NEWS | 3 +++
lib/dpdk.c | 21 +++--
vswitchd/vswitch.xml | 22 ++
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index 4985dbaac..a64b9d94d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
Post-v2.11.0
-
+ - DPDK:
+ * New option 'other_config:dpdk-socket-limit' to limit amount of
+ hugepage memory that can be used by DPDK.
v2.11.0 - xx xxx
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 0546191a6..53b74fba4 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -100,8 +100,9 @@ construct_dpdk_options(const struct smap
*ovs_other_config, struct svec *args)
bool default_enabled;
const char *default_value;
} opts[] = {
- {"dpdk-lcore-mask", "-c", false, NULL},
- {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
+ {"dpdk-lcore-mask", "-c", false, NULL},
+ {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
+ {"dpdk-socket-limit", "--socket-limit", false, NULL},
};
int i;
@@ -318,6 +319,22 @@ dpdk_init__(const struct smap *ovs_other_config)
svec_add(, ovs_get_program_name());
construct_dpdk_args(ovs_other_config, );
+ if (!args_contains(, "--legacy-mem")
+ && !args_contains(, "--socket-limit")) {
+ const char *arg;
+ size_t i;
+
+ SVEC_FOR_EACH (i, arg, ) {
+ if (!strcmp(arg, "--socket-mem")) {
+ break;
+ }
+ }
+ if (i < args.n - 1) {
+ svec_add(, "--socket-limit");
+ svec_add(, args.names[i + 1]);
+ }
+ }
+
if (args_contains(, "-c") || args_contains(, "-l")) {
auto_determine = false;
}
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 88edb5d35..1db4e8694 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -333,6 +333,28 @@
+ >>> + type='{"type": "string"}'>
+
+ Limits the maximum amount of memory that can be used from the
+ hugepage pool, on a per-socket basis.
+
+
+ The specifier is a comma-separated list of memory limits per
socket.
+ 0 will disable the limit for a particular socket.
+
+
+ If not specified, OVS will configure limits equal to the amount
of
+ preallocated memory specified by >>> + key="dpdk-socket-mem"/> or --socket-mem in
+ . If none of the
above
+ options specified or --legacy-mem provided in
+ , limits will not
be
+ applied.
+ Changing this value requires restarting the daemon.
+
+
+
>>>
>>> As the number for memory configuration options grow in OVS DPDK, do you
>>> think it makes sense to expand
Re: [ovs-dev] [PATCH v2] lib/tc: Support optional tunnel id
Thanks Roi,
On Thu, 31 Jan 2019 at 15:52, Roi Dayan wrote:
>
>
> On 31/01/2019 15:32, Roi Dayan wrote:
> >
> > On 31/01/2019 11:58, Simon Horman wrote:
> >> On Mon, Jan 21, 2019 at 05:32:37PM +0200, Adi Nissim wrote:
> >>> Currently the TC tunnel_key action is always
> >>> initialized with the given tunnel id value. However,
> >>> some tunneling protocols define the tunnel id as an optional field.
> >>>
> >>> This patch initializes the id field of tunnel_key:set and
> tunnel_key:unset
> >>> only if a value is provided.
> >>>
> >>> In the case that a tunnel key value is not provided by the user
> >>> the key flag will not be set.
> >>>
> >>> Signed-off-by: Adi Nissim
> >>> Acked-by: Paul Blakey
> >>> ---
> >>> v1->v2: check if mask.tunnel.id == OVS_BE64_MAX
> >>> so we won't do match in the case of a partial mask.
> >> I am still a bit concerned about the partial mask case.
> >> It looks like the code will now silently not offload such matches.
> >>
> >> I think that a partial mask should either be offloaded or
> >> offload of the entire flow should be rejected.
> > thanks. you are right. I didn't notice it. partial masks should be
> rejected
> > to fallback to ovs dp instead of ignoring the mask.
> >
>
>
> Hi Simon,
>
> I did some checks and think the correct fix is to offload exact match.
> if key is partial we can ignore the mask and offload exact match and
> it will be correct as we do more strict matching.
>
> it is also documented that the kernel datapath is doing the same
> (from datapath.rst)
>
> "The kernel can ignore the mask attribute, installing an exact
> match flow"
>
> So I think the first patch V0 is actually correct as we
> check the tunnel key flag exists and offload exact match if
> there was any mask or offload without a key if the mask is 0
> or no key.
>
> in netdev-tc-offloads.c
>
> +flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
> + tnl_mask->tun_id : 0;
>
I think this is fine so long as tun_id is all-ones. Is that always the case?
Should the code check that it is the case? Am I missing the point?
>
>
> in tc.c
>
> -nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> +if (id_mask) {
> +nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> +}
>
>
> let me know what you think.
>
> Thanks,
> Roi
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/2] dpdk: Limit DPDK memory usage.
On 1/31/2019 2:17 PM, Ilya Maximets wrote:
On 31.01.2019 15:39, Ian Stokes wrote:
On 1/29/2019 8:11 AM, Ilya Maximets wrote:
Since 18.05 release, DPDK moved to dynamic memory model in which
hugepages could be allocated on demand. At the same time '--socket-mem'
option was re-defined as a size of pre-allocated memory, i.e. memory
that should be allocated at startup and could not be freed.
So, DPDK with a new memory model could allocate more hugepage memory
than specified in '--socket-mem' or '-m' options.
This change adds new configurable 'other_config:dpdk-socket-limit'
which could be used to limit the ammount of memory DPDK could use.
It uses new DPDK option '--socket-limit'.
Ex.:
ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-limit="1024,1024"
Also, in order to preserve old behaviour, if '--socket-limit' is not
specified, it will be defaulted to the amount of memory specified by
'--socket-mem' option, i.e. OVS will not be able to allocate more.
This is needed, for example, to disallow OVS to allocate more memory
than reserved for it by Nova in OpenStack installations.
Signed-off-by: Ilya Maximets
Thanks for this Ilya. Given that this will affect 2.11 as well, it probably
makes sense to back port it (as well as the svec patch in this series) to OVS
2.11 so as to ensure uniform memory behavior between 2.10 to 2.11.
Yes. This probably makes sense.
Great, I've pushed to master and backported to 2.11.
One comment below also.
---
NEWS | 3 +++
lib/dpdk.c | 21 +++--
vswitchd/vswitch.xml | 22 ++
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index 4985dbaac..a64b9d94d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
Post-v2.11.0
-
+ - DPDK:
+ * New option 'other_config:dpdk-socket-limit' to limit amount of
+ hugepage memory that can be used by DPDK.
v2.11.0 - xx xxx
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 0546191a6..53b74fba4 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -100,8 +100,9 @@ construct_dpdk_options(const struct smap *ovs_other_config,
struct svec *args)
bool default_enabled;
const char *default_value;
} opts[] = {
- {"dpdk-lcore-mask", "-c", false, NULL},
- {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
+ {"dpdk-lcore-mask", "-c", false, NULL},
+ {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
+ {"dpdk-socket-limit", "--socket-limit", false, NULL},
};
int i;
@@ -318,6 +319,22 @@ dpdk_init__(const struct smap *ovs_other_config)
svec_add(, ovs_get_program_name());
construct_dpdk_args(ovs_other_config, );
+ if (!args_contains(, "--legacy-mem")
+ && !args_contains(, "--socket-limit")) {
+ const char *arg;
+ size_t i;
+
+ SVEC_FOR_EACH (i, arg, ) {
+ if (!strcmp(arg, "--socket-mem")) {
+ break;
+ }
+ }
+ if (i < args.n - 1) {
+ svec_add(, "--socket-limit");
+ svec_add(, args.names[i + 1]);
+ }
+ }
+
if (args_contains(, "-c") || args_contains(, "-l")) {
auto_determine = false;
}
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 88edb5d35..1db4e8694 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -333,6 +333,28 @@
+
+
+ Limits the maximum amount of memory that can be used from the
+ hugepage pool, on a per-socket basis.
+
+
+ The specifier is a comma-separated list of memory limits per socket.
+ 0 will disable the limit for a particular socket.
+
+
+ If not specified, OVS will configure limits equal to the amount of
+ preallocated memory specified by or --socket-mem in
+ . If none of the above
+ options specified or --legacy-mem provided in
+ , limits will not be
+ applied.
+ Changing this value requires restarting the daemon.
+
+
+
As the number for memory configuration options grow in OVS DPDK, do you think
it makes sense to expand Documentation/topics/dpdk/memory.rst to provide more
details and usage regarding these options?
I'm not sure. Some options already described in
Documentation/intro/install/dpdk.rst,
all the options described in man pages (vswitch.xml). Documentation becomes
more and
more fragmented.
I'd agree, the vswitch.xml really should be the first stop.
For instance when testing I spotted there's an eal error message for when both
--legacy and --socket-lim values are set. It's handled at the DPDK level so I
don't think we need to specifically check in OVS code but would it be worth
mentioning that dpdk-socket-limit will not be used if --legacy-mem is also
passed.
I mentioned the --legacy-mem case in man pages (vswitch.xml)
[ovs-dev] [RFC 3/3] dpdk: Silently ignore unsupported options for EAL arguments.
Remove all the warnings and silently ignores all the old configuration
knobs.
Signed-off-by: Ilya Maximets
---
lib/dpdk.c | 35 ---
1 file changed, 35 deletions(-)
diff --git a/lib/dpdk.c b/lib/dpdk.c
index fe271ce84..7585729ab 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -91,39 +91,6 @@ args_contains(const struct svec *args, const char *value)
return false;
}
-static void
-report_unsupported_configs(const struct smap *ovs_other_config)
-{
-struct option {
-const char *opt;
-const char *replacement;
-} options[] = {
-{ "dpdk-alloc-mem","-m" },
-{ "dpdk-socket-mem", "--socket-mem" },
-{ "dpdk-socket-limit", "--socket-limit" },
-{ "dpdk-lcore-mask", "-c" },
-{ "dpdk-hugepage-dir", "--huge-dir" },
-{ "dpdk-extra","" }
-};
-int i;
-bool found = false;
-
-for (i = 0; i < ARRAY_SIZE(options); i++) {
-const char *value = smap_get(ovs_other_config, options[i].opt);
-
-if (value) {
-VLOG_WARN("Detected unsupported '%s' config. Use '%s %s'"
- " in 'dpdk-options' instead. Value ignored.",
- options[i].opt, options[i].replacement, value);
-found = true;
-}
-}
-if (found) {
-VLOG_WARN("Unsupported options will be "
- "silently ignored in the future.");
-}
-}
-
static ssize_t
dpdk_log_write(void *c OVS_UNUSED, const char *buf, size_t size)
{
@@ -216,8 +183,6 @@ dpdk_init__(const struct smap *ovs_other_config)
VLOG_INFO("Per port memory for DPDK devices %s.",
per_port_memory ? "enabled" : "disabled");
-report_unsupported_configs(ovs_other_config);
-
svec_add(, ovs_get_program_name());
dpdk_options = smap_get(ovs_other_config, "dpdk-options");
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC 2/3] dpdk: Drop support of deprecated options for EAL arguments.
Drop support of all the old configuration knobs. Values are always
ignored now. Warning appears in case some of the old options detected
in database.
Signed-off-by: Ilya Maximets
---
NEWS | 2 +-
lib/dpdk.c | 155 +++--
2 files changed, 9 insertions(+), 148 deletions(-)
diff --git a/NEWS b/NEWS
index 1c09e9d57..07528da95 100644
--- a/NEWS
+++ b/NEWS
@@ -3,7 +3,7 @@ Post-v2.11.0
- DPDK:
* New option 'other_config:dpdk-options' to pass all the
DPDK EAL argumants in a single string.
- * Following config options deprecated:
+ * Following config options no longer supported:
'dpdk-alloc-mem', 'dpdk-socket-mem', 'dpdk-socket-limit',
'dpdk-lcore-mask', 'dpdk-hugepage-dir' and 'dpdk-extra'.
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 0c37f231d..fe271ce84 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -91,8 +91,8 @@ args_contains(const struct svec *args, const char *value)
return false;
}
-static bool
-report_deprecated_configs(const struct smap *ovs_other_config, bool ignore)
+static void
+report_unsupported_configs(const struct smap *ovs_other_config)
{
struct option {
const char *opt;
@@ -112,152 +112,16 @@ report_deprecated_configs(const struct smap
*ovs_other_config, bool ignore)
const char *value = smap_get(ovs_other_config, options[i].opt);
if (value) {
-VLOG_WARN("Detected deprecated '%s' config. Use '%s %s'"
- " in 'dpdk-options' instead.%s",
- options[i].opt, options[i].replacement, value,
- ignore ? " Value ignored." : "");
+VLOG_WARN("Detected unsupported '%s' config. Use '%s %s'"
+ " in 'dpdk-options' instead. Value ignored.",
+ options[i].opt, options[i].replacement, value);
found = true;
}
}
if (found) {
-VLOG_WARN("Deprecated options will be "
+VLOG_WARN("Unsupported options will be "
"silently ignored in the future.");
}
-return found;
-}
-
-static void
-construct_dpdk_options(const struct smap *ovs_other_config, struct svec *args)
-{
-struct dpdk_options_map {
-const char *ovs_configuration;
-const char *dpdk_option;
-bool default_enabled;
-const char *default_value;
-} opts[] = {
-{"dpdk-lcore-mask", "-c", false, NULL},
-{"dpdk-hugepage-dir", "--huge-dir", false, NULL},
-{"dpdk-socket-limit", "--socket-limit", false, NULL},
-};
-
-int i;
-
-/*First, construct from the flat-options (non-mutex)*/
-for (i = 0; i < ARRAY_SIZE(opts); ++i) {
-const char *value = smap_get(ovs_other_config,
- opts[i].ovs_configuration);
-if (!value && opts[i].default_enabled) {
-value = opts[i].default_value;
-}
-
-if (value) {
-if (!args_contains(args, opts[i].dpdk_option)) {
-svec_add(args, opts[i].dpdk_option);
-svec_add(args, value);
-} else {
-VLOG_WARN("Ignoring database defined option '%s' due to "
- "dpdk-extra config", opts[i].dpdk_option);
-}
-}
-}
-}
-
-static char *
-construct_dpdk_socket_mem(void)
-{
-const char *def_value = "1024";
-int numa, numa_nodes = ovs_numa_get_n_numas();
-struct ds dpdk_socket_mem = DS_EMPTY_INITIALIZER;
-
-if (numa_nodes == 0 || numa_nodes == OVS_NUMA_UNSPEC) {
-numa_nodes = 1;
-}
-
-ds_put_cstr(_socket_mem, def_value);
-for (numa = 1; numa < numa_nodes; ++numa) {
-ds_put_format(_socket_mem, ",%s", def_value);
-}
-
-return ds_cstr(_socket_mem);
-}
-
-#define MAX_DPDK_EXCL_OPTS 10
-
-static void
-construct_dpdk_mutex_options(const struct smap *ovs_other_config,
- struct svec *args)
-{
-char *default_dpdk_socket_mem = construct_dpdk_socket_mem();
-
-struct dpdk_exclusive_options_map {
-const char *category;
-const char *ovs_dpdk_options[MAX_DPDK_EXCL_OPTS];
-const char *eal_dpdk_options[MAX_DPDK_EXCL_OPTS];
-const char *default_value;
-int default_option;
-} excl_opts[] = {
-{"memory type",
- {"dpdk-alloc-mem", "dpdk-socket-mem", NULL,},
- {"-m", "--socket-mem",NULL,},
- default_dpdk_socket_mem, 1
-},
-};
-
-int i;
-for (i = 0; i < ARRAY_SIZE(excl_opts); ++i) {
-int found_opts = 0, scan, found_pos = -1;
-const char *found_value;
-struct dpdk_exclusive_options_map *popt = _opts[i];
-
-for (scan = 0; scan < MAX_DPDK_EXCL_OPTS
- && popt->ovs_dpdk_options[scan]; ++scan) {
-const char *value = smap_get(ovs_other_config,
- popt->ovs_dpdk_options[scan]);
-
[ovs-dev] [RFC 1/3] dpdk: Unify passing of DPDK EAL arguments.
This patch deprecates the usage of current configuration knobs for DPDK EAL arguments: - dpdk-alloc-mem - dpdk-socket-mem - dpdk-socket-limit - dpdk-lcore-mask - dpdk-hugepage-dir - dpdk-extra All above configuration options replaced with single 'dpdk-options' which has same format as old 'dpdk-extra', i.e. just a string with all the DPDK EAL arguments. All the documentation about old configuration knobs removed. Users could still use an old interface, but the deprecation warning will be printed. In case 'dpdk-options' provided, values of old options will be ignored. New users will start using new 'dpdk-options' as this is the only documented way providing EAL arguments. Rationale: >From release to release DPDK becomes more complex. New EAL arguments appears, old arguments becomes deprecated. Some changes their meaning and behaviour. It's not easy task to manage all this and current code in OVS that tries to manage DPDK options is not flexible/extendable enough even to track all the dependencies between options in DPDK 18.11. For example, '--socket-mem' changed its meaning, '--legacy-mem' and '--socket-limit' appeared. But '--legacy-mem' and '--socket-limit' could not be used at the same time and, also, we want to provide good default value for '--socket-limit' to keep the consistent behaviour of OVS memory usage. And this will be worse in the future. Another point is that even now we have mutually exclusive database configs in OVS and we have to handle them. i.e we're providing 'dpdk-alloc-mem' and 'dpdk-socket-mem' at the same time. Additionally, user allowed to configure same options in 'dpdk-extra'. So, we have similar/equal options in a three places in ovsdb and we need to choose which one to use. It's pretty easy for user to get into inconsistent database configuration, because users could update one field without removing others, and OVS has to resolve all the conflicts somehow constructing the EAL args. But we do not know in practice, if the resulted arguments are the arguments that user wanted to see or just forget to remove the higher priority knob. Next point is that we have 'dpdk-lcore-mask' (-c) which is actually not so user-friendly as '-l', but we have no option for it. Will we create additional 'dpdk-lcore-list' ? I guess, no, because it's not really important thing. But users will stuck with this not so user-friendly masks. Another thing is that OVS is not really need to check the consistency of the EAL arguments because DPDK could check it instead. DPDK will always check the args and it will do it better. There is no real need to duplicate this functionality. Keeping all the options in a single string 'dpdk-options' allows: * To have only one place for all the configs, i.e. it will be harder for user to forget to remove something from the single string while updating it. * Not to check the consistency between different configs, DPDK could check the cmdline itself. * Not to document every DPDK EAL argument in OVS. We can just describe few of them and point to DPDK docs for more information. * OVS still able to provide some minimal default arguments. Thanks to DPDK 18.11 we only need to specify an lcore. All other arguments are not necessary. (We still also providing default --socket-limit in case --socket-mem passed without it and without --legacy-mem) Usage example: ovs-vsctl set Open_vSwitch . \ other_config:dpdk-options="-l 1 -n 4 --socket-mem 1024,1024" Signed-off-by: Ilya Maximets --- Documentation/intro/install/dpdk.rst | 40 +--- NEWS | 7 +- lib/dpdk.c | 49 +- utilities/ovs-dev.py | 2 +- vswitchd/vswitch.xml | 132 ++- 5 files changed, 108 insertions(+), 122 deletions(-) diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index 344d2b3a6..2243fde53 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -235,32 +235,44 @@ listed below. Defaults will be provided for all values not explicitly set. A value of ``try`` will imply that the ovs-vswitchd process should continue running even if the EAL initialization fails. -``dpdk-lcore-mask`` - Specifies the CPU cores on which dpdk lcore threads should be spawned and - expects hex string (eg '0x123'). +``dpdk-options`` + Specifies the string of EAL command line arguments for DPDK. + For example, ``other_config:dpdk-options="-l 0 --socket-mem 1024,1024"``. + Important arguments that could be passed there are: -``dpdk-socket-mem`` - Comma separated list of memory to pre-allocate from hugepages on specific - sockets. If not specified, 1024 MB will be set for each numa node by - default. + - ``-c`` or ``-l`` -``dpdk-hugepage-dir`` - Directory where hugetlbfs is mounted +Specifies the CPU cores on which dpdk lcore threads should be
[ovs-dev] [RFC 0/3] dpdk: Unification/deprecation of DPDK EAL config knobs.
This patch-set deprecates the usage of current configuration knobs for DPDK EAL arguments: - dpdk-alloc-mem - dpdk-socket-mem - dpdk-socket-limit - dpdk-lcore-mask - dpdk-hugepage-dir - dpdk-extra All above configuration options replaced with single 'dpdk-options' which has same format as old 'dpdk-extra', i.e. just a string with all the DPDK EAL arguments. Rationale: See the commit message of the patch #1. This patch set contains 3 patches that describes some kind of the deprecation process. Patch 1: Introduces 'dpdk-options' and removes all the documentation about old configuration knobs. Users could still use an old interface, but the deprecation warning will be printed. In case 'dpdk-options' provided, values of old options will be ignored. New users will start using new 'dpdk-options' as this is the only documented way providing EAL arguments. This patch could be applied now. Patch 2: Drops support of all the old configuration knobs. Values are always ignored now. Warning appears in case some of the old options detected in database. This could be applied after branching the 2.12. Patch 3: Removes all the warnings and silently ignores all the old configuration knobs. This could be applied after branching the 2.13. This allows to fully drop all the old options in a year. Patch-set based on previous '--socket-limit' work: https://patchwork.ozlabs.org/patch/1032583/ Ilya Maximets (3): dpdk: Unify passing of DPDK EAL arguments. dpdk: Drop support of deprecated options for EAL arguments. dpdk: Silently ignore unsupported options for EAL arguments. Documentation/intro/install/dpdk.rst | 40 +--- NEWS | 7 +- lib/dpdk.c | 143 ++- utilities/ovs-dev.py | 2 +- vswitchd/vswitch.xml | 132 ++--- 5 files changed, 68 insertions(+), 256 deletions(-) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
We have hit this issue as well on 2.9, would it be possible to backport it? On Mon, Oct 8, 2018 at 7:17 PM Ben Pfaff wrote: > > On Fri, Oct 05, 2018 at 09:19:54AM -0700, Yi-Hung Wei wrote: > > The patch address vswitchd crash when it receives NXT_RESUME with geneve > > tunnel metadata. The crash is due to segmentation fault with the > > following stack trace, and it is observed only in kernel datapath. > > A test is added to prevent regression. > > > > Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault. > > 0 0x7fcffd0c5412 in tun_metadata_to_geneve__ > > (flow=flow@entry=0x7ffcb7106680, b=b@entry=0x7ffcb70eb5a8, > > crit_opt=crit_opt@entry=0x7ffcb70eb287) > >at lib/tun-metadata.c:676 > > 1 0x7fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow > > (b=0x7ffcb70eb5a8, flow=0x7ffcb7106638) at lib/tun-metadata.c:706 > > 2 tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638, > > flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry=0x7ffcb70eb5a8) > >at lib/tun-metadata.c:810 > > 3 0x7fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8, > > tun_key=tun_key@entry=0x7ffcb7106638, > > tun_flow_key=tun_flow_key@entry=0x7ffcb7106638, > >key_buf=key_buf@entry=0x0, tnl_type=, tnl_type@entry=0x0) > > at lib/odp-util.c:2886 > > 4 0x7fcffd0551cf in odp_key_from_dp_packet > > (buf=buf@entry=0x7ffcb70eb5a8, packet=0x7ffcb7106590) at lib/odp-util.c:5909 > > 5 0x7fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8, > > d_exec=0x7ffcb7106428, dp_ifindex=) at > > lib/dpif-netlink.c:1873 > > 6 dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00, > > ops=ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at > > lib/dpif-netlink.c:1959 > > 7 0x7fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1, > > ops=0x7ffcb7106418, dpif=) at lib/dpif-netlink.c:2258 > > 8 dpif_netlink_operate (dpif_=0xe65e00, ops=, > > n_ops=) at lib/dpif-netlink.c:2294 > > 9 0x7fcffd014680 in dpif_operate (dpif=, ops= > out>, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif.c:1359 > > 10 0x7fcffd014c58 in dpif_execute (dpif=, > > execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324 > > 11 0x7fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50, pin=0x7ffcb7107150) > > at ofproto/ofproto-dpif.c:4885 > > 12 0x7fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0, > > oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612 > > 13 0x7fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0) > > at ofproto/ofproto.c:8137 > > 14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at > > ofproto/ofproto.c:8258 > > 15 0x7fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0 > > , ofconn=0xf8c8f0) at ofproto/connmgr.c:1432 > > 16 connmgr_run (mgr=0xe422f0, > > handle_openflow=handle_openflow@entry=0x7fcffd4046f0 ) at > > ofproto/connmgr.c:363 > > 17 0x7fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821 > > 18 0x0040ca94 in bridge_run__ () at vswitchd/bridge.c:2939 > > 19 0x00411d44 in bridge_run () at vswitchd/bridge.c:2997 > > 20 0x004094fd in main (argc=12, argv=0x7ffcb71085b8) at > > vswitchd/ovs-vswitchd.c:119 > > > > VMWare-BZ: #2210216 > > Signed-off-by: Yi-Hung Wei > > --- > > Please backport it to 2.10. > > Applied to master and branch-2.10, thanks! > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] 答复: [PATCH] conntrack: Remove unnecessary check in process_ftp_ctl_v4
发件人: Darrell Ball [mailto:dlu...@gmail.com] 发送时间: 2019年2月1日 16:15 收件人: Li,Rongqing 抄送: ovs dev 主题: Re: [ovs-dev] [PATCH] conntrack: Remove unnecessary check in process_ftp_ctl_v4 >This was intentionally done to be documentative and also make it hard to break; >this code path sees a tiny number of packets. >I am not sure there is much to gain by removing it and adding in lieu of >comments ? gain for packets is little, but step by step. And it can reduce unnessesary codes, make a newbie to easy study Thanks -RongQing ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] 答复: [PATCH] conntrack: compute hash value once when calling conn_key_lookup
发件人: Darrell Ball [mailto:dlu...@gmail.com] 发送时间: 2019年2月1日 16:25 收件人: Li,Rongqing 抄送: ovs dev 主题: Re: [ovs-dev] [PATCH] conntrack: compute hash value once when calling conn_key_lookup > The function comment says it all: > /* Typical usage of this helper is in non per-packet code; > * this is because the bucket lock needs to be held for lookup > * and a hash would have already been needed. Hence, this function > * is just intended for code clarity. */ > Does it mean that the hash value will be changed ? if it is true, should we release original lock, and relock lock for new hash value? Thanks -RongQing ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] test: Fix failed test "flow resume with geneve tun_metadata"
On Fri, Feb 1, 2019 at 12:10 AM Yifeng Sun wrote:
> Test "flow resume with geneve tun_metadata" failed because there is
> no controller running to send controller(pause) message. A previous
> commit deleted the line that starts ovs-ofctl as a controller in
> order to avoid a race condition on monitor log. This patch adds
> back this line but omits the log file because this test doesn't
> dpends on the log file.
>
> Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition
> on monitor log")
> CC: David Marchand
> Signed-off-by: Yifeng Sun
> ---
> tests/system-traffic.at | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index ffe508dd61f7..84c2af4170a3 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -538,6 +538,8 @@ OVS_CHECK_GENEVE()
> OVS_TRAFFIC_VSWITCHD_START()
> ADD_BR([br-underlay])
>
> +AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2>
> /dev/null])
> +
> ADD_NAMESPACES(at_ns0)
>
> dnl Set up underlay link from host into the namespace using veth pair.
> @@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 |
> FORMAT_PING], [0], [dnl
> 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> ])
>
> +OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
>
Oops.
Thanks for fixing.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: compute hash value once when calling conn_key_lookup
Thanks for looking
On Fri, Jan 18, 2019 at 1:47 AM Li RongQing wrote:
> it is expensive to compute hash value, and When call conn_lookup,
> hash value is computed twice, in fact, we can cache it and pass
> it to conn_lookup
>
> Signed-off-by: Zhang Yu
> Signed-off-by: Li RongQing
> ---
> lib/conntrack.c | 19 +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 11a1e05bd..f01c2ceac 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -743,12 +743,13 @@ un_nat_packet(struct dp_packet *pkt, const struct
> conn *conn,
> * and a hash would have already been needed. Hence, this function
> * is just intended for code clarity. */
> static struct conn *
> -conn_lookup(struct conntrack *ct, const struct conn_key *key, long long
> now)
> +conn_lookup(struct conntrack *ct, const struct conn_key *key, long long
> now,
>
The function comment says it all:
/* Typical usage of this helper is in non per-packet code;
* this is because the bucket lock needs to be held for lookup
* and a hash would have already been needed. Hence, this function
* is just intended for code clarity. */
> +uint32_t hash)
> {
> struct conn_lookup_ctx ctx;
> ctx.conn = NULL;
> ctx.key = *key;
> -ctx.hash = conn_key_hash(key, ct->hash_basis);
> +ctx.hash = hash;
> unsigned bucket = hash_to_bucket(ctx.hash);
> conn_key_lookup(>buckets[bucket], , now);
> return ctx.conn;
> @@ -758,9 +759,10 @@ static void
> conn_seq_skew_set(struct conntrack *ct, const struct conn_key *key,
>long long now, int seq_skew, bool seq_skew_dir)
> {
> -unsigned bucket = hash_to_bucket(conn_key_hash(key, ct->hash_basis));
> +uint32_t hash = conn_key_hash(key, ct->hash_basis);
> +unsigned bucket = hash_to_bucket(hash);
> ct_lock_lock(>buckets[bucket].lock);
> -struct conn *conn = conn_lookup(ct, key, now);
> +struct conn *conn = conn_lookup(ct, key, now, hash);
> if (conn && seq_skew) {
> conn->seq_skew = seq_skew;
> conn->seq_skew_dir = seq_skew_dir;
> @@ -777,12 +779,13 @@ nat_clean(struct conntrack *ct, struct conn *conn,
> nat_conn_keys_remove(>nat_conn_keys, >rev_key,
> ct->hash_basis);
> ct_rwlock_unlock(>resources_lock);
> ct_lock_unlock(>lock);
> -unsigned bucket_rev_conn =
> -hash_to_bucket(conn_key_hash(>rev_key, ct->hash_basis));
> +
> +uint32_t hash = conn_key_hash(>rev_key, ct->hash_basis);
> +unsigned bucket_rev_conn = hash_to_bucket(hash);
> ct_lock_lock(>buckets[bucket_rev_conn].lock);
> ct_rwlock_wrlock(>resources_lock);
> long long now = time_msec();
> -struct conn *rev_conn = conn_lookup(ct, >rev_key, now);
> +struct conn *rev_conn = conn_lookup(ct, >rev_key, now, hash);
> struct nat_conn_key_node *nat_conn_key_node =
> nat_conn_keys_lookup(>nat_conn_keys, >rev_key,
> ct->hash_basis);
> @@ -1016,7 +1019,7 @@ create_un_nat_conn(struct conntrack *ct, struct conn
> *conn_for_un_nat_copy,
> uint32_t un_nat_hash = conn_key_hash(>key, ct->hash_basis);
> unsigned un_nat_conn_bucket = hash_to_bucket(un_nat_hash);
> ct_lock_lock(>buckets[un_nat_conn_bucket].lock);
> -struct conn *rev_conn = conn_lookup(ct, >key, now);
> +struct conn *rev_conn = conn_lookup(ct, >key, now, un_nat_hash);
>
> if (alg_un_nat) {
> if (!rev_conn) {
> --
> 2.16.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Remove unnecessary check in process_ftp_ctl_v4
On Fri, Jan 18, 2019 at 1:49 AM Li RongQing wrote:
> It has been assured that both first and second int from ftp
> command are not bigger than 255, so their combination(first
> int << 8 +second int) must not bigger than 65535
>
> Signed-off-by: Wang Li
> Signed-off-by: Li RongQing
> ---
> lib/conntrack.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 6f6021a97..11a1e05bd 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2987,12 +2987,7 @@ process_ftp_ctl_v4(struct conntrack *ct,
> return CT_FTP_CTL_INVALID;
> }
>
> -uint16_t port_lo_hs = value;
> -if (65535 - port_hs < port_lo_hs) {
> -return CT_FTP_CTL_INVALID;
> -}
> -
> -port_hs |= port_lo_hs;
> +port_hs |= value;
>
This was intentionally done to be documentative and also make it hard to
break;
this code path sees a tiny number of packets.
I am not sure there is much to gain by removing it and adding in lieu of
comments ?
> ovs_be16 port = htons(port_hs);
> ovs_be32 conn_ipv4_addr;
>
> --
> 2.16.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Can the ovs source code be modified???
Hi. I installed ovs using mininet. Mininet version 2.3.0, ovs version 2.5.5. I want to log information about the Packet whenever it arrives in ovs. What should I do?? Also, when the FlowTable is full, I want to delete the FlowEntry by using the interval time between packets. How can I do this?? Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
