[ovs-dev] dringende Antwort erforderlich
Schöne Grüße, Ich bin Mr.Patrick William von einem globalen Kreditkreditunternehmen, das als Global Reliance Credit® bekannt ist. Wir bieten alle Arten von Darlehen zu einem Zinssatz von 1% an. Wenn Sie ein Darlehen benötigen, kontaktieren Sie uns bitte mit den untenstehenden Informationen. Bitte füllen Sie das untenstehende Formular aus und senden Sie es so schnell wie möglich zurück. Vollständiger Name: Geschlecht: Benötigter Betrag: Dauer: Wir freuen uns, Sie zu unterstützen. Kontaktieren Sie uns per E-Mail: i...@globalreliancecredits.com Verwaltung Mr. Patrick William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v2] conntrack: fix ftp ipv4 address substitution.
Fixed a bug introduced in V3 and added a second patch and sent V4.
i.e. V3 is superceded
On Mon, Jan 21, 2019 at 4:22 PM Darrell Ball wrote:
> Ignore V2; I sent V3 with more improvements.
>
> On Mon, Jan 21, 2019 at 12:12 PM Darrell Ball wrote:
>
>> From: David Marchand
>>
>> When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
>> was incorrectly calculated which could lead to the wrong replacement
>> adjustment.
>>
>> This goes unnoticed most of the time, unless you choose carefully your
>> initial and replacement addresses.
>>
>> Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
>>
>> Fix this by getting rid of remain_size since it is a derived variable
>> and use the primary variable byte_str instead.
>>
>> A test is updated to exercise different initial and replacement addresses
>> and another test is added.
>>
>> Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
>> Co-authored-by: Darrell Ball
>> Signed-off-by: Darrell Ball
>> Signed-off-by: David Marchand
>> ---
>> lib/conntrack.c | 8 ++
>> tests/system-traffic.at | 74
>> -
>> 2 files changed, 70 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index f732b9e..f1e6ee2 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -2806,8 +2806,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
>> v4_addr_rep,
>> return 0;
>> }
>>
>> -size_t remain_size = tcp_payload_length(pkt) -
>> - addr_offset_from_ftp_data_start;
>> int overall_delta = 0;
>> char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
>>
>> @@ -2817,13 +2815,13 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
>> v4_addr_rep,
>> char *next_delim = memchr(byte_str, ',', 4);
>> ovs_assert(next_delim);
>> int substr_size = next_delim - byte_str;
>> -remain_size -= substr_size;
>>
>> /* Compose the new string for this octet, and replace it. */
>> -char rep_str[4];
>> uint8_t rep_byte = get_v4_byte_be(v4_addr_rep, i);
>> +char rep_str[4];
>> int replace_size = sprintf(rep_str, "%d", rep_byte);
>> -replace_substring(byte_str, substr_size, remain_size,
>> +replace_substring(byte_str, substr_size,
>> + (const char *) dp_packet_tail(pkt) - byte_str,
>>rep_str, replace_size);
>> overall_delta += replace_size - substr_size;
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 3a62e17..3e3b9e7 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -4652,6 +4652,66 @@
>> tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
>> OVS_TRAFFIC_VSWITCHD_STOP
>> AT_CLEANUP
>>
>> +AT_SETUP([conntrack - IPv4 FTP Passive with DNAT 2])
>> +AT_SKIP_IF([test $HAVE_FTP = no])
>> +CHECK_CONNTRACK()
>> +CHECK_CONNTRACK_NAT()
>> +CHECK_CONNTRACK_ALG()
>> +
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +
>> +ADD_NAMESPACES(at_ns0, at_ns1)
>> +
>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/16")
>> +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address e6:66:c1:11:11:11])
>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.200 e6:66:c1:22:22:22])
>> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.100.1 e6:66:c1:22:22:22])
>> +
>> +ADD_VETH(p1, at_ns1, br0, "10.1.100.1/16")
>> +NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address e6:66:c1:22:22:22])
>> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e6:66:c1:11:11:11])
>> +
>> +dnl Allow any traffic from ns0->ns1.
>> +AT_DATA([flows.txt], [dnl
>> +dnl track all IPv4 traffic and NAT any established traffic.
>> +table=0 priority=10 ip, action=ct(nat,table=1)
>> +table=0 priority=0 action=drop
>> +dnl
>> +dnl Table 1
>> +dnl
>> +dnl Allow new FTP control connections.
>> +table=1 in_port=1 ct_state=+new tcp nw_src=10.1.1.1 tp_dst=21
>> action=ct(alg=ftp,commit,nat(dst=10.1.100.1)),2
>> +dnl Allow related TCP connections from port 1.
>> +table=1 in_port=1 ct_state=+new+rel tcp nw_src=10.1.1.1
>> action=ct(commit,nat),2
>> +dnl Allow established TCP connections both ways, post-NAT match.
>> +table=1 in_port=1 ct_state=+est tcp nw_dst=10.1.100.1 action=2
>> +table=1 in_port=2 ct_state=+est tcp nw_dst=10.1.1.1 action=1
>> +
>> +dnl Allow ICMP both ways.
>> +table=1 priority=100 in_port=1 icmp, action=2
>> +table=1 priority=100 in_port=2 icmp, action=1
>> +table=1 priority=0, action=drop
>> +])
>> +
>> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
>> +
>> +dnl Check that the stacks working to avoid races.
>> +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.100.1 >/dev/null])
>> +
>> +OVS_START_L7([at_ns1], [ftp])
>> +
>> +dnl FTP requests from p0->p1 should work fine.
>> +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.200 -t 3 -T 1
>> --retry-connrefused -v -o wget0.log])
>> +
>> +dnl Discards CLOSE_WAIT and CLOSING
>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack |
[ovs-dev] [patch v4 2/2] conntrack: Fix max size for inet_ntop() call.
The call to inet_ntop() in repl_ftp_v6_addr() is 1 short to handle
the maximum possible V6 address size for v4 mapping case.
Found by inspection.
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 6b66750..922a58c 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3161,7 +3161,7 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr
v6_addr_rep,
char v6_addr_str[IPV6_SCAN_LEN] = {0};
ovs_assert(inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str,
- IPV6_SCAN_LEN - 1));
+ sizeof v6_addr_str));
modify_packet(pkt, ftp_data_start + addr_offset_from_ftp_data_start,
addr_size, v6_addr_str, strlen(v6_addr_str),
orig_used_size);
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v4 1/2] conntrack: fix ftp ipv4 address substitution.
When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
was incorrectly calculated which could lead to the wrong replacement
adjustment.
This goes unnoticed most of the time, unless you choose carefully your
initial and replacement addresses.
Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
Fix this by doing something similar to V6 and also splicing out common
code for better coverage and maintainability.
A test is updated to exercise different initial and replacement addresses
and another test is added.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: David Marchand
Signed-off-by: David Marchand
Signed-off-by: Darrell Ball
---
v4: Fix bug in call to inet_ntop() in repl_ftp_v4_addr().
v3: Make V4 and V6 code paths more common.
v2: Minimum alternative patch; also add tests.
lib/conntrack.c | 118 +++-
tests/system-traffic.at | 74 +++---
2 files changed, 123 insertions(+), 69 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f732b9e..6b66750 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -136,7 +136,7 @@ expectation_lookup(struct hmap *alg_expectations, const
struct conn_key *key,
static int
repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep,
char *ftp_data_v4_start,
- size_t addr_offset_from_ftp_data_start);
+ size_t addr_offset_from_ftp_data_start, size_t addr_size);
static enum ftp_ctl_pkt
process_ftp_ctl_v4(struct conntrack *ct,
@@ -144,7 +144,8 @@ process_ftp_ctl_v4(struct conntrack *ct,
const struct conn *conn_for_expectation,
ovs_be32 *v4_addr_rep,
char **ftp_data_v4_start,
- size_t *addr_offset_from_ftp_data_start);
+ size_t *addr_offset_from_ftp_data_start,
+ size_t *addr_size);
static enum ftp_ctl_pkt
detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
@@ -2771,13 +2772,6 @@ expectation_create(struct conntrack *ct, ovs_be16
dst_port,
ct_rwlock_unlock(&ct->resources_lock);
}
-static uint8_t
-get_v4_byte_be(ovs_be32 v4_addr, uint8_t index)
-{
-uint8_t *byte_ptr = (OVS_FORCE uint8_t *) &v4_addr;
-return byte_ptr[index];
-}
-
static void
replace_substring(char *substr, uint8_t substr_size,
uint8_t total_size, char *rep_str,
@@ -2788,51 +2782,56 @@ replace_substring(char *substr, uint8_t substr_size,
memcpy(substr, rep_str, rep_str_size);
}
+static void
+repl_bytes(char *str, char c1, char c2)
+{
+while (*str) {
+if (*str == c1) {
+*str = c2;
+}
+str++;
+}
+}
+
+static void
+modify_packet(struct dp_packet *pkt, char *pkt_str, size_t size,
+ char *repl_str, size_t repl_size,
+ uint32_t orig_used_size)
+{
+replace_substring(pkt_str, size,
+ (const char *) dp_packet_tail(pkt) - pkt_str,
+ repl_str, repl_size);
+dp_packet_set_size(pkt, orig_used_size + (int) repl_size - (int) size);
+}
+
/* Replace IPV4 address in FTP message with NATed address. */
static int
repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep,
char *ftp_data_start,
- size_t addr_offset_from_ftp_data_start)
+ size_t addr_offset_from_ftp_data_start,
+ size_t addr_size OVS_UNUSED)
{
enum { MAX_FTP_V4_NAT_DELTA = 8 };
/* Do conservative check for pathological MTU usage. */
uint32_t orig_used_size = dp_packet_size(pkt);
-uint16_t allocated_size = dp_packet_get_allocated(pkt);
-if (orig_used_size + MAX_FTP_V4_NAT_DELTA > allocated_size) {
+if (orig_used_size + MAX_FTP_V4_NAT_DELTA >
+dp_packet_get_allocated(pkt)) {
+
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
-VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP",
- allocated_size);
+VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP V4",
+ dp_packet_get_allocated(pkt));
return 0;
}
-size_t remain_size = tcp_payload_length(pkt) -
- addr_offset_from_ftp_data_start;
-int overall_delta = 0;
-char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
-
-/* Replace the existing IPv4 address by the new one. */
-for (uint8_t i = 0; i < 4; i++) {
-/* Find the end of the string for this octet. */
-char *next_delim = memchr(byte_str, ',', 4);
-ovs_assert(next_delim);
-int substr_size = next_delim - byte_str;
-remain_size -= substr_size;
-
-/* Compose the new string for this octet, and replace it. */
-char rep_str[4];
-uint8_t rep_byte = get_v4_byte_be(v4_addr_rep, i);
-int replace_size = sprintf(rep_str, "%d", rep_b
[ovs-dev] New Order
Good day, Please arrange to provide the best offer for below attached Purchase Order The requirement for our green field contract in Oman Kindly get back to us 1) Proforma invoice with bank details 2) Delivery date 3) FOB/CIF Port Regards, kahn Gotze Sales & Services Assistant 1 attachment PO367459.xls (72K) Preview download ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [branch-2.9 2/3] conntrack: fix expectations for ftp+DNAT.
From: David Marchand
When configuring the nat part of an expectation, care must be taken to
look at the master nat action and direction to properly reproduce it.
DNAT tests have been added to both active and passive modes, all
ftp/tftp tests titles have been updated to reflect they are dealing with
SNAT.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball
Signed-off-by: Darrell Ball
Signed-off-by: David Marchand
---
lib/conntrack.c | 12 ++-
tests/system-traffic.at | 222 +++-
2 files changed, 211 insertions(+), 23 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index c3881c7..7b46576 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2639,21 +2639,29 @@ expectation_create(struct conntrack *ct, ovs_be16
dst_port,
if (reply) {
src_addr = master_conn->key.src.addr;
dst_addr = master_conn->key.dst.addr;
+alg_exp_node->nat_rpl_dst = true;
if (skip_nat) {
alg_nat_repl_addr = dst_addr;
+} else if (master_conn->nat_info &&
+ master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+alg_nat_repl_addr = master_conn->rev_key.src.addr;
+alg_exp_node->nat_rpl_dst = false;
} else {
alg_nat_repl_addr = master_conn->rev_key.dst.addr;
}
-alg_exp_node->nat_rpl_dst = true;
} else {
src_addr = master_conn->rev_key.src.addr;
dst_addr = master_conn->rev_key.dst.addr;
+alg_exp_node->nat_rpl_dst = false;
if (skip_nat) {
alg_nat_repl_addr = src_addr;
+} else if (master_conn->nat_info &&
+ master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+alg_nat_repl_addr = master_conn->key.dst.addr;
+alg_exp_node->nat_rpl_dst = true;
} else {
alg_nat_repl_addr = master_conn->key.src.addr;
}
-alg_exp_node->nat_rpl_dst = false;
}
if (src_ip_wc) {
memset(&src_addr, 0, sizeof src_addr);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 755faa3..66d3b1d 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3341,7 +3341,7 @@ dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, using the provided flow table.
m4_define([CHECK_FTP_NAT],
- [AT_SETUP([conntrack - FTP NAT $1])
+ [AT_SETUP([conntrack - FTP $1])
AT_SKIP_IF([test $HAVE_FTP = no])
AT_SKIP_IF([test $HAVE_LFTP = no])
CHECK_CONNTRACK()
@@ -3383,7 +3383,7 @@ ls
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP])
-dnl CHECK_FTP_NAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, with flow tables that implement the NATing as part of handling of
@@ -3391,8 +3391,8 @@ dnl initial incoming packets - ie, the first flow is
ct(nat,table=foo).
dnl
dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_PRE_RECIRC], [dnl
- CHECK_FTP_NAT([prerecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_PRE_RECIRC], [dnl
+CHECK_FTP_NAT([SNAT prerecirc $1], [$2], [dnl
dnl track all IP traffic, de-mangle non-NEW connections
table=0 in_port=1, ip, action=ct(table=1,nat)
table=0 in_port=2, ip, action=ct(table=2,nat)
@@ -3446,7 +3446,7 @@
tcp,orig=(src=10.1.1.2,dst=$2,sport=,dport=),reply=(src=10.1.1
])
dnl Check that ct(nat,table=foo) works without TCP sequence adjustment.
-CHECK_FTP_NAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
+CHECK_FTP_SNAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
dnl Check that ct(nat,table=foo) works with TCP sequence adjustment.
dnl
@@ -3457,9 +3457,9 @@ dnl of 10.1.1.1 used in the test and 10.1.1.240 here),
the FTP NAT ALG must
dnl resize the packet and adjust TCP sequence numbers. This test is kept
dnl separate from the above to easier identify issues in this code on different
dnl kernels.
-CHECK_FTP_NAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
+CHECK_FTP_SNAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
-dnl CHECK_FTP_NAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, with flow tables that implement the NATing after the first round
@@ -3468,8 +3468,8 @@ dnl flow will implement the NATing with
ct(nat..),output:foo.
dnl
dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_POST_RECIRC], [dnl
-CHECK_FTP_NAT([postrecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_POST_RECIRC], [dnl
+CHECK_FTP_NAT([SNAT postrec
[ovs-dev] [branch-2.9 3/3] conntrack: Fix FTP seq_skew boundary adjustments.
At the same time, splice out a function and also rely on the compiler
for overflow/underflow handling.
Found by inspection.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 38 ++
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 7b46576..1cb5f01 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3104,6 +3104,13 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr
v6_addr_rep,
return overall_delta;
}
+/* Increment/decrement a TCP sequence number. */
+static void
+adj_seqnum(ovs_16aligned_be32 *val, int32_t inc)
+{
+put_16aligned_be32(val, htonl(ntohl(get_16aligned_be32(val)) + inc));
+}
+
static void
handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
struct dp_packet *pkt, const struct conn *ec, long long now,
@@ -3178,34 +3185,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
struct tcp_header *th = dp_packet_l4(pkt);
if (nat && ec->seq_skew != 0) {
-if (ctx->reply != ec->seq_skew_dir) {
-
-uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
-
-if ((ec->seq_skew > 0) && (tcp_ack < ec->seq_skew)) {
-/* Should not be possible; will be marked invalid. */
-tcp_ack = 0;
-} else if ((ec->seq_skew < 0) &&
- (UINT32_MAX - tcp_ack < -ec->seq_skew)) {
-tcp_ack = (-ec->seq_skew) - (UINT32_MAX - tcp_ack);
-} else {
-tcp_ack -= ec->seq_skew;
-}
-ovs_be32 new_tcp_ack = htonl(tcp_ack);
-put_16aligned_be32(&th->tcp_ack, new_tcp_ack);
-} else {
-uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq));
-if ((ec->seq_skew > 0) && (UINT32_MAX - tcp_seq < ec->seq_skew)) {
-tcp_seq = ec->seq_skew - (UINT32_MAX - tcp_seq);
-} else if ((ec->seq_skew < 0) && (tcp_seq < -ec->seq_skew)) {
-/* Should not be possible; will be marked invalid. */
-tcp_seq = 0;
-} else {
-tcp_seq += ec->seq_skew;
-}
-ovs_be32 new_tcp_seq = htonl(tcp_seq);
-put_16aligned_be32(&th->tcp_seq, new_tcp_seq);
-}
+ctx->reply != ec->seq_skew_dir ?
+adj_seqnum(&th->tcp_ack, -ec->seq_skew) :
+adj_seqnum(&th->tcp_seq, ec->seq_skew);
}
th->tcp_csum = 0;
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [branch-2.9 1/3] conntrack: fix tcp seq adjustments when mangling commands.
From: David Marchand
The ftp alg deals with packets in two ways for the command connection:
either they are inspected and can be mangled when nat is enabled
(CT_FTP_CTL_INTEREST) or they just go through without being modified
(CT_FTP_CTL_OTHER).
For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
number by the connection current offset, then prepare for the next
packets by setting an accumulated offset in the ct object. However,
this was not done for multiple CT_FTP_CTL_INTEREST packets for the same
connection.
This is relevant for handling multiple child data connections that also
need natting.
The tests are updated so that some ftp+NAT tests send multiple port
commands or other similar commands for a single control connection.
Wget is not able to do this, so switch to lftp.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball
Signed-off-by: Darrell Ball
Signed-off-by: David Marchand
---
Vagrantfile | 19 ++--
Vagrantfile-FreeBSD | 2 +-
lib/conntrack.c | 77 -
tests/atlocal.in| 3 ++
tests/system-traffic.at | 14 -
5 files changed, 71 insertions(+), 44 deletions(-)
diff --git a/Vagrantfile b/Vagrantfile
index 28e9219..d7e38a1 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -12,7 +12,8 @@ dnf -y install autoconf automake openssl-devel libtool \
python-twisted python-zope-interface \
desktop-file-utils groff graphviz rpmdevtools nc curl \
wget python-six pyftpdlib checkpolicy selinux-policy-devel \
- libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy
+ libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy \
+ lftp
echo "search extra update built-in" >/etc/depmod.d/search_path.conf
SCRIPT
@@ -20,6 +21,7 @@ $bootstrap_debian = <
Re: [ovs-dev] conntrack: fix tcp seq adjustments when mangling commands.
I am going to guess it is due to space vs '-' i.e. s/branch 2.9/branch-2.9/ and resend On Mon, Jan 21, 2019 at 6:59 PM 0-day Robot wrote: > Bleep bloop. Greetings Darrell Ball, 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 conntrack: fix tcp seq adjustments when mangling > commands. > 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] conntrack: fix tcp seq adjustments when mangling commands.
Bleep bloop. Greetings Darrell Ball, 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 conntrack: fix tcp seq adjustments when mangling commands. 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
[ovs-dev] [branch 2.9 2/3] conntrack: fix expectations for ftp+DNAT.
From: David Marchand
When configuring the nat part of an expectation, care must be taken to
look at the master nat action and direction to properly reproduce it.
DNAT tests have been added to both active and passive modes, all
ftp/tftp tests titles have been updated to reflect they are dealing with
SNAT.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball
Signed-off-by: Darrell Ball
Signed-off-by: David Marchand
---
lib/conntrack.c | 12 ++-
tests/system-traffic.at | 222 +++-
2 files changed, 211 insertions(+), 23 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index c3881c7..7b46576 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2639,21 +2639,29 @@ expectation_create(struct conntrack *ct, ovs_be16
dst_port,
if (reply) {
src_addr = master_conn->key.src.addr;
dst_addr = master_conn->key.dst.addr;
+alg_exp_node->nat_rpl_dst = true;
if (skip_nat) {
alg_nat_repl_addr = dst_addr;
+} else if (master_conn->nat_info &&
+ master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+alg_nat_repl_addr = master_conn->rev_key.src.addr;
+alg_exp_node->nat_rpl_dst = false;
} else {
alg_nat_repl_addr = master_conn->rev_key.dst.addr;
}
-alg_exp_node->nat_rpl_dst = true;
} else {
src_addr = master_conn->rev_key.src.addr;
dst_addr = master_conn->rev_key.dst.addr;
+alg_exp_node->nat_rpl_dst = false;
if (skip_nat) {
alg_nat_repl_addr = src_addr;
+} else if (master_conn->nat_info &&
+ master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+alg_nat_repl_addr = master_conn->key.dst.addr;
+alg_exp_node->nat_rpl_dst = true;
} else {
alg_nat_repl_addr = master_conn->key.src.addr;
}
-alg_exp_node->nat_rpl_dst = false;
}
if (src_ip_wc) {
memset(&src_addr, 0, sizeof src_addr);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 755faa3..66d3b1d 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3341,7 +3341,7 @@ dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, using the provided flow table.
m4_define([CHECK_FTP_NAT],
- [AT_SETUP([conntrack - FTP NAT $1])
+ [AT_SETUP([conntrack - FTP $1])
AT_SKIP_IF([test $HAVE_FTP = no])
AT_SKIP_IF([test $HAVE_LFTP = no])
CHECK_CONNTRACK()
@@ -3383,7 +3383,7 @@ ls
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP])
-dnl CHECK_FTP_NAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, with flow tables that implement the NATing as part of handling of
@@ -3391,8 +3391,8 @@ dnl initial incoming packets - ie, the first flow is
ct(nat,table=foo).
dnl
dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_PRE_RECIRC], [dnl
- CHECK_FTP_NAT([prerecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_PRE_RECIRC], [dnl
+CHECK_FTP_NAT([SNAT prerecirc $1], [$2], [dnl
dnl track all IP traffic, de-mangle non-NEW connections
table=0 in_port=1, ip, action=ct(table=1,nat)
table=0 in_port=2, ip, action=ct(table=2,nat)
@@ -3446,7 +3446,7 @@
tcp,orig=(src=10.1.1.2,dst=$2,sport=,dport=),reply=(src=10.1.1
])
dnl Check that ct(nat,table=foo) works without TCP sequence adjustment.
-CHECK_FTP_NAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
+CHECK_FTP_SNAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
dnl Check that ct(nat,table=foo) works with TCP sequence adjustment.
dnl
@@ -3457,9 +3457,9 @@ dnl of 10.1.1.1 used in the test and 10.1.1.240 here),
the FTP NAT ALG must
dnl resize the packet and adjust TCP sequence numbers. This test is kept
dnl separate from the above to easier identify issues in this code on different
dnl kernels.
-CHECK_FTP_NAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
+CHECK_FTP_SNAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
-dnl CHECK_FTP_NAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
dnl
dnl Checks the implementation of conntrack with FTP ALGs in combination with
dnl NAT, with flow tables that implement the NATing after the first round
@@ -3468,8 +3468,8 @@ dnl flow will implement the NATing with
ct(nat..),output:foo.
dnl
dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_POST_RECIRC], [dnl
-CHECK_FTP_NAT([postrecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_POST_RECIRC], [dnl
+CHECK_FTP_NAT([SNAT postrec
[ovs-dev] [branch 2.9 1/3] conntrack: fix tcp seq adjustments when mangling commands.
From: David Marchand
The ftp alg deals with packets in two ways for the command connection:
either they are inspected and can be mangled when nat is enabled
(CT_FTP_CTL_INTEREST) or they just go through without being modified
(CT_FTP_CTL_OTHER).
For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
number by the connection current offset, then prepare for the next
packets by setting an accumulated offset in the ct object. However,
this was not done for multiple CT_FTP_CTL_INTEREST packets for the same
connection.
This is relevant for handling multiple child data connections that also
need natting.
The tests are updated so that some ftp+NAT tests send multiple port
commands or other similar commands for a single control connection.
Wget is not able to do this, so switch to lftp.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball
Signed-off-by: Darrell Ball
Signed-off-by: David Marchand
---
Vagrantfile | 19 ++--
Vagrantfile-FreeBSD | 2 +-
lib/conntrack.c | 77 -
tests/atlocal.in| 3 ++
tests/system-traffic.at | 14 -
5 files changed, 71 insertions(+), 44 deletions(-)
diff --git a/Vagrantfile b/Vagrantfile
index 28e9219..d7e38a1 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -12,7 +12,8 @@ dnf -y install autoconf automake openssl-devel libtool \
python-twisted python-zope-interface \
desktop-file-utils groff graphviz rpmdevtools nc curl \
wget python-six pyftpdlib checkpolicy selinux-policy-devel \
- libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy
+ libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy \
+ lftp
echo "search extra update built-in" >/etc/depmod.d/search_path.conf
SCRIPT
@@ -20,6 +21,7 @@ $bootstrap_debian = <
[ovs-dev] [branch 2.9 3/3] conntrack: Fix FTP seq_skew boundary adjustments.
At the same time, splice out a function and also rely on the compiler
for overflow/underflow handling.
Found by inspection.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 38 ++
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 7b46576..1cb5f01 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3104,6 +3104,13 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr
v6_addr_rep,
return overall_delta;
}
+/* Increment/decrement a TCP sequence number. */
+static void
+adj_seqnum(ovs_16aligned_be32 *val, int32_t inc)
+{
+put_16aligned_be32(val, htonl(ntohl(get_16aligned_be32(val)) + inc));
+}
+
static void
handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
struct dp_packet *pkt, const struct conn *ec, long long now,
@@ -3178,34 +3185,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
struct tcp_header *th = dp_packet_l4(pkt);
if (nat && ec->seq_skew != 0) {
-if (ctx->reply != ec->seq_skew_dir) {
-
-uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
-
-if ((ec->seq_skew > 0) && (tcp_ack < ec->seq_skew)) {
-/* Should not be possible; will be marked invalid. */
-tcp_ack = 0;
-} else if ((ec->seq_skew < 0) &&
- (UINT32_MAX - tcp_ack < -ec->seq_skew)) {
-tcp_ack = (-ec->seq_skew) - (UINT32_MAX - tcp_ack);
-} else {
-tcp_ack -= ec->seq_skew;
-}
-ovs_be32 new_tcp_ack = htonl(tcp_ack);
-put_16aligned_be32(&th->tcp_ack, new_tcp_ack);
-} else {
-uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq));
-if ((ec->seq_skew > 0) && (UINT32_MAX - tcp_seq < ec->seq_skew)) {
-tcp_seq = ec->seq_skew - (UINT32_MAX - tcp_seq);
-} else if ((ec->seq_skew < 0) && (tcp_seq < -ec->seq_skew)) {
-/* Should not be possible; will be marked invalid. */
-tcp_seq = 0;
-} else {
-tcp_seq += ec->seq_skew;
-}
-ovs_be32 new_tcp_seq = htonl(tcp_seq);
-put_16aligned_be32(&th->tcp_seq, new_tcp_seq);
-}
+ctx->reply != ec->seq_skew_dir ?
+adj_seqnum(&th->tcp_ack, -ec->seq_skew) :
+adj_seqnum(&th->tcp_seq, ec->seq_skew);
}
th->tcp_csum = 0;
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] seq: Correct example in comment.
It was deceptive for the example to imply that a seq can be declared
directly, because the API only allows for creating a new one on the heap.
Signed-off-by: Ben Pfaff
---
lib/seq.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/seq.h b/lib/seq.h
index 221ab9acddc5..92743c1eb4ea 100644
--- a/lib/seq.h
+++ b/lib/seq.h
@@ -77,14 +77,14 @@
*
*struct ovs_mutex mutex;
*struct ovs_list queue OVS_GUARDED_BY(mutex);
- *struct seq nonempty_seq;
+ *struct seq *nonempty_seq;
*
* To add an element to the queue:
*
*ovs_mutex_lock(&mutex);
*ovs_list_push_back(&queue, ...element...);
*if (ovs_list_is_singleton(&queue)) { // The 'if' test here is optional.
- *seq_change(&nonempty_seq);
+ *seq_change(nonempty_seq);
*}
*ovs_mutex_unlock(&mutex);
*
@@ -92,7 +92,7 @@
*
*ovs_mutex_lock(&mutex);
*if (ovs_list_is_empty(&queue)) {
- *seq_wait(&nonempty_seq, seq_read(&nonempty_seq));
+ *seq_wait(nonempty_seq, seq_read(nonempty_seq));
*} else {
*poll_immediate_wake();
*}
--
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v2] conntrack: fix ftp ipv4 address substitution.
Ignore V2; I sent V3 with more improvements.
On Mon, Jan 21, 2019 at 12:12 PM Darrell Ball wrote:
> From: David Marchand
>
> When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
> was incorrectly calculated which could lead to the wrong replacement
> adjustment.
>
> This goes unnoticed most of the time, unless you choose carefully your
> initial and replacement addresses.
>
> Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
>
> Fix this by getting rid of remain_size since it is a derived variable
> and use the primary variable byte_str instead.
>
> A test is updated to exercise different initial and replacement addresses
> and another test is added.
>
> Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> Co-authored-by: Darrell Ball
> Signed-off-by: Darrell Ball
> Signed-off-by: David Marchand
> ---
> lib/conntrack.c | 8 ++
> tests/system-traffic.at | 74
> -
> 2 files changed, 70 insertions(+), 12 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f732b9e..f1e6ee2 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2806,8 +2806,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
> v4_addr_rep,
> return 0;
> }
>
> -size_t remain_size = tcp_payload_length(pkt) -
> - addr_offset_from_ftp_data_start;
> int overall_delta = 0;
> char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
>
> @@ -2817,13 +2815,13 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
> v4_addr_rep,
> char *next_delim = memchr(byte_str, ',', 4);
> ovs_assert(next_delim);
> int substr_size = next_delim - byte_str;
> -remain_size -= substr_size;
>
> /* Compose the new string for this octet, and replace it. */
> -char rep_str[4];
> uint8_t rep_byte = get_v4_byte_be(v4_addr_rep, i);
> +char rep_str[4];
> int replace_size = sprintf(rep_str, "%d", rep_byte);
> -replace_substring(byte_str, substr_size, remain_size,
> +replace_substring(byte_str, substr_size,
> + (const char *) dp_packet_tail(pkt) - byte_str,
>rep_str, replace_size);
> overall_delta += replace_size - substr_size;
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 3a62e17..3e3b9e7 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -4652,6 +4652,66 @@
> tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([conntrack - IPv4 FTP Passive with DNAT 2])
> +AT_SKIP_IF([test $HAVE_FTP = no])
> +CHECK_CONNTRACK()
> +CHECK_CONNTRACK_NAT()
> +CHECK_CONNTRACK_ALG()
> +
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +ADD_NAMESPACES(at_ns0, at_ns1)
> +
> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/16")
> +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address e6:66:c1:11:11:11])
> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.200 e6:66:c1:22:22:22])
> +NS_CHECK_EXEC([at_ns0], [arp -s 10.1.100.1 e6:66:c1:22:22:22])
> +
> +ADD_VETH(p1, at_ns1, br0, "10.1.100.1/16")
> +NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address e6:66:c1:22:22:22])
> +NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e6:66:c1:11:11:11])
> +
> +dnl Allow any traffic from ns0->ns1.
> +AT_DATA([flows.txt], [dnl
> +dnl track all IPv4 traffic and NAT any established traffic.
> +table=0 priority=10 ip, action=ct(nat,table=1)
> +table=0 priority=0 action=drop
> +dnl
> +dnl Table 1
> +dnl
> +dnl Allow new FTP control connections.
> +table=1 in_port=1 ct_state=+new tcp nw_src=10.1.1.1 tp_dst=21
> action=ct(alg=ftp,commit,nat(dst=10.1.100.1)),2
> +dnl Allow related TCP connections from port 1.
> +table=1 in_port=1 ct_state=+new+rel tcp nw_src=10.1.1.1
> action=ct(commit,nat),2
> +dnl Allow established TCP connections both ways, post-NAT match.
> +table=1 in_port=1 ct_state=+est tcp nw_dst=10.1.100.1 action=2
> +table=1 in_port=2 ct_state=+est tcp nw_dst=10.1.1.1 action=1
> +
> +dnl Allow ICMP both ways.
> +table=1 priority=100 in_port=1 icmp, action=2
> +table=1 priority=100 in_port=2 icmp, action=1
> +table=1 priority=0, action=drop
> +])
> +
> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +
> +dnl Check that the stacks working to avoid races.
> +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.100.1 >/dev/null])
> +
> +OVS_START_L7([at_ns1], [ftp])
> +
> +dnl FTP requests from p0->p1 should work fine.
> +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.200 -t 3 -T 1
> --retry-connrefused -v -o wget0.log])
> +
> +dnl Discards CLOSE_WAIT and CLOSING
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.200)], [0],
> [dnl
>
> +tcp,orig=(src=10.1.1.1,dst=10.1.1.200,sport=,dport=),reply=(src=10.1.100.1,dst=10.1.1.1,sport=,dport=),protoinfo=(state=)
>
> +tcp,orig=(src=10.1.1.1,dst=10.1.1.200,sport=,dport=),reply=(src=10.1.100.1,dst=10.1.1.1,sport=,dport=),pr
[ovs-dev] [patch v3] conntrack: fix ftp ipv4 address substitution.
When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
was incorrectly calculated which could lead to the wrong replacement
adjustment.
This goes unnoticed most of the time, unless you choose carefully your
initial and replacement addresses.
Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
Fix this by doing something similar to V6 and also splicing out common
code for better coverage and maintainability.
A test is updated to exercise different initial and replacement addresses
and another test is added.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: David Marchand
Signed-off-by: David Marchand
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 119 +++-
tests/system-traffic.at | 74 +++---
2 files changed, 124 insertions(+), 69 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f732b9e..dd5b3e3 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -136,7 +136,7 @@ expectation_lookup(struct hmap *alg_expectations, const
struct conn_key *key,
static int
repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep,
char *ftp_data_v4_start,
- size_t addr_offset_from_ftp_data_start);
+ size_t addr_offset_from_ftp_data_start, size_t addr_size);
static enum ftp_ctl_pkt
process_ftp_ctl_v4(struct conntrack *ct,
@@ -144,7 +144,8 @@ process_ftp_ctl_v4(struct conntrack *ct,
const struct conn *conn_for_expectation,
ovs_be32 *v4_addr_rep,
char **ftp_data_v4_start,
- size_t *addr_offset_from_ftp_data_start);
+ size_t *addr_offset_from_ftp_data_start,
+ size_t *addr_size);
static enum ftp_ctl_pkt
detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
@@ -2771,13 +2772,6 @@ expectation_create(struct conntrack *ct, ovs_be16
dst_port,
ct_rwlock_unlock(&ct->resources_lock);
}
-static uint8_t
-get_v4_byte_be(ovs_be32 v4_addr, uint8_t index)
-{
-uint8_t *byte_ptr = (OVS_FORCE uint8_t *) &v4_addr;
-return byte_ptr[index];
-}
-
static void
replace_substring(char *substr, uint8_t substr_size,
uint8_t total_size, char *rep_str,
@@ -2788,51 +2782,57 @@ replace_substring(char *substr, uint8_t substr_size,
memcpy(substr, rep_str, rep_str_size);
}
+static void
+repl_bytes(char *str, char c1, char c2)
+{
+while (*str) {
+if (*str == c1) {
+*str = c2;
+}
+str++;
+}
+}
+
+static void
+modify_packet(struct dp_packet *pkt, char *pkt_str, size_t size,
+ char *repl_str, size_t repl_size,
+ uint32_t orig_used_size)
+{
+replace_substring(pkt_str, size,
+ (const char *) dp_packet_tail(pkt) - pkt_str,
+ repl_str, repl_size);
+dp_packet_set_size(pkt, orig_used_size + (int) repl_size - (int) size);
+}
+
/* Replace IPV4 address in FTP message with NATed address. */
static int
repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep,
char *ftp_data_start,
- size_t addr_offset_from_ftp_data_start)
+ size_t addr_offset_from_ftp_data_start,
+ size_t addr_size OVS_UNUSED)
{
enum { MAX_FTP_V4_NAT_DELTA = 8 };
/* Do conservative check for pathological MTU usage. */
uint32_t orig_used_size = dp_packet_size(pkt);
-uint16_t allocated_size = dp_packet_get_allocated(pkt);
-if (orig_used_size + MAX_FTP_V4_NAT_DELTA > allocated_size) {
+if (orig_used_size + MAX_FTP_V4_NAT_DELTA >
+dp_packet_get_allocated(pkt)) {
+
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
-VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP",
- allocated_size);
+VLOG_WARN_RL(&rl, "Unsupported effective MTU %u used with FTP V4",
+ dp_packet_get_allocated(pkt));
return 0;
}
-size_t remain_size = tcp_payload_length(pkt) -
- addr_offset_from_ftp_data_start;
-int overall_delta = 0;
-char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
-
-/* Replace the existing IPv4 address by the new one. */
-for (uint8_t i = 0; i < 4; i++) {
-/* Find the end of the string for this octet. */
-char *next_delim = memchr(byte_str, ',', 4);
-ovs_assert(next_delim);
-int substr_size = next_delim - byte_str;
-remain_size -= substr_size;
-
-/* Compose the new string for this octet, and replace it. */
-char rep_str[4];
-uint8_t rep_byte = get_v4_byte_be(v4_addr_rep, i);
-int replace_size = sprintf(rep_str, "%d", rep_byte);
-replace_substring(byte_str, substr_size, remain_size,
- rep_str, replace_size);
-overall_delta += repla
Re: [ovs-dev] [PATCH] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels
On 1/21/2019 2:06 PM, Gregory Rose wrote: It turns out after I had more of a chance to look at this that HAVE_INET_FRAGS_WITH_FRAGS_WORK and HAVE_INET_FRAGS_RND are both keying off the same upstream patch I mentioned in the commit message (648700f76b03). There's no need for the redundancy. Nice catch! I'm going to fix this by getting rid of HAVE_INET_FRAGS_WITH_FRAGS_WORK because that is sort of unwieldy and only used in a few spots. We can simplify acinclude.m4 a bit and work with a single define for the compilation switch. A v2 patch will be forthcoming. Thanks for the review! I take that back. If you go far enough back (3.16.1) then you see that the frags_work field is added independently of the rnd field in the inet_frags structure. It is correct to separately check for frags_work and rnd as done in this patch because there are times when one field will be defined but not the other. It is true that upstream commit 648700f76b03 removed the frags_work and rnd fields at the same time but they were introduced separately and require discrete compilation flags. If there are no other objections can I please get an ack? Thanks, - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] compat: Fixup ipv6 fragmentation on 4.9.135+ kernels
On 1/14/2019 4:20 PM, Gregory Rose wrote:
On 1/14/2019 3:22 PM, Yi-Hung Wei wrote:
On Thu, Jan 10, 2019 at 2:09 PM Greg Rose wrote:
Upstream commit 648700f76b03 ("inet: frags: use rhashtables...")
changed
how ipv6 fragmentation is implemented. This patch was backported to
the upstream stable 4.9.x kernel starting at 4.9.135.
This patch creates the compatibility layer changes required to both
compile and also operate correctly with ipv6 fragmentation on these
kernels. Check if the inet_frags 'rnd' field is present to key on
whether the upstream patch is present. Also update Travis to the
latest 4.9 kernel release so that this patch is compile tested.
Passes Travis:
https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409
Cc: William Tu
Cc: Yi-Hung Wei
Cc: Yifeng Sun
Signed-off-by: Greg Rose
---
Thanks Greg for the patch. In general, it looks good to me, I only
have two small questions as below. I tested it on 4.10.17 stable
kernel from linux-stable tree. The compilation is passed, and the
relevant IPv6 fragmentation system traffic tests are passed.
@@ -614,10 +658,12 @@ void ovs_netns_frags6_init(struct net *net)
void ovs_netns_frags6_exit(struct net *net)
{
+#ifdef HAVE_INET_FRAGS_RND
struct netns_frags *frags;
frags = get_netns_frags6_from_net(net);
inet_frags_exit_net(frags, &nf_frags);
+#endif
}
Don't we need to do inet_frags_exit_net() if HAVE_INET_FRAGS_RND is
false?
From ./net/ipv6/netfilter/nf_conntrack_reasm.c in the linux-stable
branch 4.10.y, it looks like inet_frags_exit_net() is still be used.
In 4.9.135 it was causing a panic. I added that at the time to
prevent the panic but perhaps there's a better
solution I should be looking for.
Yi-hung,
I looked more closely at this in the stable tree and for 4.10.y
HAVE_INET_FRAGS_RND will be defined and
this function will work as you expect. So unless I'm missing something
I feel confident that this is the correct
thing to do here.
.
-#ifdef HAVE_INET_FRAGS_WITH_FRAGS_WORK
+#if defined(HAVE_INET_FRAGS_WITH_FRAGS_WORK) ||
!defined(HAVE_INET_FRAGS_RND)
nf_frags.frags_cache_name = nf_frags_cache_name;
#endif
Not sure if this changed is needed? It seems to me that it depends on
HAVE_INET_FRAGS_WITH_FRAGS_WORK but not on HAVE_INET_FRAGS_RND?
It turns out after I had more of a chance to look at this that
HAVE_INET_FRAGS_WITH_FRAGS_WORK and
HAVE_INET_FRAGS_RND are both keying off the same upstream patch I
mentioned in the commit message
(648700f76b03). There's no need for the redundancy. Nice catch!
I'm going to fix this by getting rid of HAVE_INET_FRAGS_WITH_FRAGS_WORK
because that is sort of
unwieldy and only used in a few spots. We can simplify acinclude.m4 a
bit and work with a single define
for the compilation switch.
A v2 patch will be forthcoming. Thanks for the review!
- Greg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Google Cloud Users Contact List
Hi, Hope you're having a great day! I just wanted to know if you're looking to acquire Google Cloud Users Contact List for your marketing efforts? Information Field: Names, Title, Email, Phone, Company Name, Company URL, Company physical address, SIC Code, Industry and Company Size (Revenue and Employee). Kindly review and let me know of your target interest so that I can get back to you with the exact counts and sample file. Do let me know if you have any questions for me. Regards, Kathleen Brown Database Executive If you do not wish to receive these emails. Please respond Exit. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: fix ftp ipv4 address substitution
Thanks for the fix David
I sent an alternative fix here:
https://patchwork.ozlabs.org/patch/1028908/
Darrell
On Mon, Jan 21, 2019 at 2:15 AM David Marchand
wrote:
> replace_substring() wants the total size of the string in order to move
> the end of the string after the part being replaced.
>
> When replacing the ipv4 address in repl_ftp_v4_addr(), the remain_size
> variable must be updated after replace_substring() has been called, not
> before.
> Besides, the substraction is off by one, since we need to account for the
> ',' that is skipped.
>
> This goes unnoticed most of the time, unless you choose carefully your
> setup and ip addresses.
> Example for ftp in passive mode with address 10.1.1.200 DNAT'd to
> 10.1.100.1:
>
> Breakpoint 1, replace_substring (substr=0x1e68497
> "10,1,100,1,241,144).\r\n", substr_size=2 '\002', total_size=20 '\024',
> rep_str=0x7fff92ee4e50 "10", rep_str_size=2 '\002') at
> lib/conntrack.c:2864
>
> Breakpoint 1, replace_substring (substr=0x1e6849a
> "1,100,1,241,144).\r\n", substr_size=1 '\001', total_size=19 '\023',
> rep_str=0x7fff92ee4e50 "1", rep_str_size=1 '\001') at
> lib/conntrack.c:2864
>
> Breakpoint 1, replace_substring (substr=0x1e6849c "100,1,241,144).\r\n",
> substr_size=3 '\003', total_size=16 '\020', rep_str=0x7fff92ee4e50 "1",
> rep_str_size=1 '\001') at lib/conntrack.c:2864
>
> Breakpoint 1, replace_substring (substr=0x1e6849e "1,241,144).\r.\r\n",
> substr_size=1 '\001', total_size=15 '\017', rep_str=0x7fff92ee4e50
> "200", rep_str_size=3 '\003') at lib/conntrack.c:2864
>
> From the "unnated" side, the payload ends up being incorrectly terminated:
>
> 0x0040: b715 3232 3720 456e 7465 7269 6e67 2050 ..227.Entering.P
> 0x0050: 6173 7369 7665 204d 6f64 6520 2831 302c assive.Mode.(10,
> 0x0060: 312c 312c 3230 302c 3234 312c 3134 3429 1,1,200,241,144)
> 0x0070: 2e0d 2e ...
>
> Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> Signed-off-by: David Marchand
> ---
> lib/conntrack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f732b9e..8eddc8e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2817,7 +2817,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
> v4_addr_rep,
> char *next_delim = memchr(byte_str, ',', 4);
> ovs_assert(next_delim);
> int substr_size = next_delim - byte_str;
> -remain_size -= substr_size;
>
> /* Compose the new string for this octet, and replace it. */
> char rep_str[4];
> @@ -2825,6 +2824,7 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
> v4_addr_rep,
> int replace_size = sprintf(rep_str, "%d", rep_byte);
> replace_substring(byte_str, substr_size, remain_size,
>rep_str, replace_size);
> +remain_size -= substr_size + 1;
> overall_delta += replace_size - substr_size;
>
> /* Advance past the octet and the following comma. */
> --
> 1.8.3.1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v2] conntrack: fix ftp ipv4 address substitution.
From: David Marchand
When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
was incorrectly calculated which could lead to the wrong replacement
adjustment.
This goes unnoticed most of the time, unless you choose carefully your
initial and replacement addresses.
Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
Fix this by getting rid of remain_size since it is a derived variable
and use the primary variable byte_str instead.
A test is updated to exercise different initial and replacement addresses
and another test is added.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball
Signed-off-by: Darrell Ball
Signed-off-by: David Marchand
---
lib/conntrack.c | 8 ++
tests/system-traffic.at | 74 -
2 files changed, 70 insertions(+), 12 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f732b9e..f1e6ee2 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2806,8 +2806,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
v4_addr_rep,
return 0;
}
-size_t remain_size = tcp_payload_length(pkt) -
- addr_offset_from_ftp_data_start;
int overall_delta = 0;
char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
@@ -2817,13 +2815,13 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
v4_addr_rep,
char *next_delim = memchr(byte_str, ',', 4);
ovs_assert(next_delim);
int substr_size = next_delim - byte_str;
-remain_size -= substr_size;
/* Compose the new string for this octet, and replace it. */
-char rep_str[4];
uint8_t rep_byte = get_v4_byte_be(v4_addr_rep, i);
+char rep_str[4];
int replace_size = sprintf(rep_str, "%d", rep_byte);
-replace_substring(byte_str, substr_size, remain_size,
+replace_substring(byte_str, substr_size,
+ (const char *) dp_packet_tail(pkt) - byte_str,
rep_str, replace_size);
overall_delta += replace_size - substr_size;
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 3a62e17..3e3b9e7 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4652,6 +4652,66 @@
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - IPv4 FTP Passive with DNAT 2])
+AT_SKIP_IF([test $HAVE_FTP = no])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+CHECK_CONNTRACK_ALG()
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/16")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address e6:66:c1:11:11:11])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.200 e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.100.1 e6:66:c1:22:22:22])
+
+ADD_VETH(p1, at_ns1, br0, "10.1.100.1/16")
+NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e6:66:c1:11:11:11])
+
+dnl Allow any traffic from ns0->ns1.
+AT_DATA([flows.txt], [dnl
+dnl track all IPv4 traffic and NAT any established traffic.
+table=0 priority=10 ip, action=ct(nat,table=1)
+table=0 priority=0 action=drop
+dnl
+dnl Table 1
+dnl
+dnl Allow new FTP control connections.
+table=1 in_port=1 ct_state=+new tcp nw_src=10.1.1.1 tp_dst=21
action=ct(alg=ftp,commit,nat(dst=10.1.100.1)),2
+dnl Allow related TCP connections from port 1.
+table=1 in_port=1 ct_state=+new+rel tcp nw_src=10.1.1.1 action=ct(commit,nat),2
+dnl Allow established TCP connections both ways, post-NAT match.
+table=1 in_port=1 ct_state=+est tcp nw_dst=10.1.100.1 action=2
+table=1 in_port=2 ct_state=+est tcp nw_dst=10.1.1.1 action=1
+
+dnl Allow ICMP both ways.
+table=1 priority=100 in_port=1 icmp, action=2
+table=1 priority=100 in_port=2 icmp, action=1
+table=1 priority=0, action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl Check that the stacks working to avoid races.
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.100.1 >/dev/null])
+
+OVS_START_L7([at_ns1], [ftp])
+
+dnl FTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.200 -t 3 -T 1 --retry-connrefused
-v -o wget0.log])
+
+dnl Discards CLOSE_WAIT and CLOSING
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.200)], [0], [dnl
+tcp,orig=(src=10.1.1.1,dst=10.1.1.200,sport=,dport=),reply=(src=10.1.100.1,dst=10.1.1.1,sport=,dport=),protoinfo=(state=)
+tcp,orig=(src=10.1.1.1,dst=10.1.1.200,sport=,dport=),reply=(src=10.1.100.1,dst=10.1.1.1,sport=,dport=),protoinfo=(state=),helper=ftp
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([conntrack - IPv4 FTP Active with DNAT])
AT_SKIP_IF([test $HAVE_FTP = no])
CHECK_CONNTRACK()
@@ -4722,12 +4782,12 @@ OVS_TRAFFIC_VSWITCHD_START()
ADD_NAMESPACES(at_ns0, at_ns1)
-ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p0, at_ns0, br0,
[ovs-dev] Investment Proposal of € 7.5M
Hello I want to seek for your assistance and partnership to invest in your country. I have € 7.5M (Seven million five hundred thousand Euros) that I want to invest in your country and I am willing to offer you 20% of the money for your assistance in this project. If you are interested, kindly reply me as soon as you read my mail for more details. I am waiting for your response Sincerely Lisa Herrera ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v4 0/3] fixes for ftp alg in userspace dp.
On Mon, Jan 21, 2019 at 11:33:29AM +0100, David Marchand wrote: > Hello Ben, > > On Sat, Jan 19, 2019 at 1:24 AM Ben Pfaff wrote: > > > On Tue, Jan 15, 2019 at 06:58:14PM -0800, Darrell Ball wrote: > > > Some small fixes to make the ftp alg behave a little better. > > > I tried to come up with better tests but I am not too sure if we want to > > > have a dependency on yet another external tool for it (lftp). > > > > > > Changelog since v3: > > > - applied Darrell comments > > > - more child connections, little style change in patch 1 > > > - added a DNAT tests with "reverse skew" in patch 2 > > > - added patch from Darrell to fix seq/ack adjustments > > > > > > Changelog since v2: > > > - applied Darrell comments > > > > > > Changelog since v1: > > > - dropped unnecessary changes > > > - squashed code changes and tests together > > > - sticked closer to the original code > > > > Thanks for figuring this out with David. > > > > I applied this to master and backported to branch-2.10 and branch-2.11. > > Beyond that it failed to apply cleanly, so please submit backported > > patches if it needs to go farther. > > > > I will be out this week, but I can send those patches on 2.9 next week, if > this is okay with you. Works for me. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC] stt: fix return code during xmit
On 1/16/2019 8:03 AM, Aaron Conole wrote: Following code looks like it might be wrong. I don't know much about the way the stt infrastructure is being used, so feel free to ignore if it is expected to return NETDEV_TX_OK even in error cases (just seems strange). Caught by compiler warning: /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c: In function ‘ovs_stt_xmit’: /home/travis/build/ovsrobot/ovs/datapath/linux/stt.c:1005:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable] int err; ^ If not used, then consider alternatively just dropping the variable. Signed-off-by: Aaron Conole --- diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c index 506f1d90a..5f045120e 100644 --- a/datapath/linux/compat/stt.c +++ b/datapath/linux/compat/stt.c @@ -1029,7 +1029,7 @@ netdev_tx_t ovs_stt_xmit(struct sk_buff *skb) error: kfree_skb(skb); dev->stats.tx_errors++; - return NETDEV_TX_OK; + return err; } EXPORT_SYMBOL(ovs_stt_xmit); Looks good to me. I don't know if you need to resend without the RFC tag or not but in any case... Reviewed-by: Greg Rose ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] travis/linux-build: enable testing with clang builds
The CLANG version of the builds have not honored the TESTSUITE variable. This dates to at least 2015, and the reason for the restriction isn't clear. Signed-off-by: Aaron Conole --- .travis/linux-build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 42115b08d..0a2091061 100755 --- a/.travis/linux-build.sh +++ b/.travis/linux-build.sh @@ -119,7 +119,7 @@ else make -j2 CFLAGS="$CFLAGS $BUILD_ENV $SPARSE_FLAGS" C=1 fi -if [ "$TESTSUITE" ] && [ "$CC" != "clang" ]; then +if [ "$TESTSUITE" ]; then if ! make distcheck TESTSUITEFLAGS=-j4 RECHECK=yes; then # testsuite.log is necessary for debugging. cat */_build/tests/testsuite.log -- 2.19.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/2] travis: enable testsuite with dpdk
The testsuite flag isn't currently being passed for DPDK. Let's pass it and when a future DPDK supports running the check-dpdk suite, we can turn that on then, too. Signed-off-by: Aaron Conole --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index b74ba2bfd..5e475e22d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,6 +35,7 @@ env: - BUILD_ENV="-m32" OPTS="--disable-ssl" - KERNEL=3.16.54 DPDK=1 - KERNEL=3.16.54 DPDK=1 OPTS="--enable-shared" + - KERNEL=3.16.54 TESTSUITE=1 DPDK=1 - KERNEL=3.16.54 DPDK_SHARED=1 - KERNEL=3.16.54 DPDK_SHARED=1 OPTS="--enable-shared" - KERNEL=4.17.14 -- 2.19.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/2] travis: misc. improvements
Following are some quick changes to improve the travis testing. Aaron Conole (2): travis: enable testsuite with dpdk travis/linux-build: enable testing with clang builds .travis.yml| 1 + .travis/linux-build.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) -- 2.19.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] CFDI en 100 casos prácticos
Cursos escenciales - Webinar Interactivo – Viernes 08 de Febrero Capacitación a tu Medida Todo sobre CFDI en 100 casos prácticos El asistente conocerá los recientes cambios en materia de comprobantes fiscales digitales y su complemento de pago a fin de cerciorarse de la emisión correcta para sus clientes y los aspectos que debe cuidar para implementar el control interno correcto en el crédito y cobranza a proveedores, de acuerdo con lo previsto en el Anexo 20 y las Guías de llenado. Revisión general a los CFDI, fundamentos legales y aplicación práctica en casos específicos. Ejes Temáticos: • Marco Jurídico. • Casos prácticos de los CFDI. • Registro Contable de los CFDI. Para mayor información, responder sobre este correo con la palabra CFDI+ los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: Llamanos al (045) 55 1554 6630 www.Innovalearn.mx ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/4] dpcls subtable miniflow optimizations
Ilya Maximets writes: > On 21.01.2019 19:01, Aaron Conole wrote: >> Harry van Haaren writes: >> >>> Hi Folks, >>> >>> This patchset is a v4, changes from v3 are a fix to the issue reported >>> by Ilya (see v3 patchset for details). Note that this patchset enables >>> the work as presented at OVS Conf last December, particularly this is >>> the function pointer part: https://www.youtube.com/watch?v=5-MDlpUIOBE >> >> Hi Harry, >> >> I'm seeing quite a few failures associated with this patch set (see >> below log for an example): >> >>https://api.travis-ci.org/v3/job/482434899/log.txt > > The most of failures are because of ukey installation failures: > > 2019-01-21T15:06:03.609Z|00267|ofproto_dpif_upcall|WARN|upcall_cb > failure: ukey installation fails > > And also because some packets was missed in stats (probably dropped because > of processing issues) > >> >> Seems to be with gcc and the test suite. The clang builds don't seem to >> be impacted. > > We're not running testsuite with clang in Travis, actually, regardless of > 'TESTSUITE' variable. That is the reason of green clang builds. Okay - I'll look at a cooking up a patch (I've already started on one to enable 'DPDK=1 TESTSUITE=1'). It isn't clear currently why it should be disabled still (I realize it was for the total build time, but the travis build is currently over 2 hours... seems better to just re-enable). >> I haven't dived deep into the failures, but the tests >> which are failing. >> >>> The work contained in this patchset achieves the following; >>> >>> Patch 1: >>> Refactor dpcls_lookup and the subtable for flexibility. >>> In particular, add a function pointer to the subtable >>> structure, which enables "plugging-in" a lookup function >>> at runtime. This enables a number of optimizations in future. >>> >>> Patch 2 & 3: >>> With the function pointer in place, we refactor the existing >>> dpcls_lookup matching code into its own function, and later its >>> own file. To split it to its own file requires making various >>> dpcls data-structures available in the dpif-netdev.h header. >>> >>> Patch 4: >>> Re-implement and optimize dpcls_rule_matches_key() by removing >>> the "loopy-branch-ness" of the FOR_EACH() macros used. Instead >>> a popcount() approach is used, which is much more CPU performance >>> friendly, due to reduced branches/loads-stores and total work done. >>> >>> Performance: >>> Patches 1, 2 and 3 are performance neutral in testing here. The >>> fourth patch provides a significant performance improvement when >>> dpcls or SMC are processing packets. >>> >>> >>> Feedback, reviews, performance numbers weclomed! -Harry >>> >>> >>> Harry van Haaren (4): >>> dpif-netdev: implement function pointers/subtable >>> dpif-netdev: move dpcls lookup structures to .h >>> dpif-netdev: split out generic lookup function >>> dpif-netdev: optimized dpcls_rule_matches_key() >>> >>> lib/automake.mk | 1 + >>> lib/dpif-netdev-lookup-generic.c | 95 ++ >>> lib/dpif-netdev.c| 124 +++-- >>> lib/dpif-netdev.h| 130 +++ >>> 4 files changed, 238 insertions(+), 112 deletions(-) >>> create mode 100644 lib/dpif-netdev-lookup-generic.c >> >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/4] dpcls subtable miniflow optimizations
On 21.01.2019 19:01, Aaron Conole wrote: > Harry van Haaren writes: > >> Hi Folks, >> >> This patchset is a v4, changes from v3 are a fix to the issue reported >> by Ilya (see v3 patchset for details). Note that this patchset enables >> the work as presented at OVS Conf last December, particularly this is >> the function pointer part: https://www.youtube.com/watch?v=5-MDlpUIOBE > > Hi Harry, > > I'm seeing quite a few failures associated with this patch set (see > below log for an example): > >https://api.travis-ci.org/v3/job/482434899/log.txt The most of failures are because of ukey installation failures: 2019-01-21T15:06:03.609Z|00267|ofproto_dpif_upcall|WARN|upcall_cb failure: ukey installation fails And also because some packets was missed in stats (probably dropped because of processing issues) > > Seems to be with gcc and the test suite. The clang builds don't seem to > be impacted. We're not running testsuite with clang in Travis, actually, regardless of 'TESTSUITE' variable. That is the reason of green clang builds. > I haven't dived deep into the failures, but the tests > which are failing. > >> The work contained in this patchset achieves the following; >> >> Patch 1: >> Refactor dpcls_lookup and the subtable for flexibility. >> In particular, add a function pointer to the subtable >> structure, which enables "plugging-in" a lookup function >> at runtime. This enables a number of optimizations in future. >> >> Patch 2 & 3: >> With the function pointer in place, we refactor the existing >> dpcls_lookup matching code into its own function, and later its >> own file. To split it to its own file requires making various >> dpcls data-structures available in the dpif-netdev.h header. >> >> Patch 4: >> Re-implement and optimize dpcls_rule_matches_key() by removing >> the "loopy-branch-ness" of the FOR_EACH() macros used. Instead >> a popcount() approach is used, which is much more CPU performance >> friendly, due to reduced branches/loads-stores and total work done. >> >> Performance: >> Patches 1, 2 and 3 are performance neutral in testing here. The >> fourth patch provides a significant performance improvement when >> dpcls or SMC are processing packets. >> >> >> Feedback, reviews, performance numbers weclomed! -Harry >> >> >> Harry van Haaren (4): >> dpif-netdev: implement function pointers/subtable >> dpif-netdev: move dpcls lookup structures to .h >> dpif-netdev: split out generic lookup function >> dpif-netdev: optimized dpcls_rule_matches_key() >> >> lib/automake.mk | 1 + >> lib/dpif-netdev-lookup-generic.c | 95 ++ >> lib/dpif-netdev.c| 124 +++-- >> lib/dpif-netdev.h| 130 +++ >> 4 files changed, 238 insertions(+), 112 deletions(-) >> create mode 100644 lib/dpif-netdev-lookup-generic.c > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/4] dpcls subtable miniflow optimizations
Harry van Haaren writes: > Hi Folks, > > This patchset is a v4, changes from v3 are a fix to the issue reported > by Ilya (see v3 patchset for details). Note that this patchset enables > the work as presented at OVS Conf last December, particularly this is > the function pointer part: https://www.youtube.com/watch?v=5-MDlpUIOBE Hi Harry, I'm seeing quite a few failures associated with this patch set (see below log for an example): https://api.travis-ci.org/v3/job/482434899/log.txt Seems to be with gcc and the test suite. The clang builds don't seem to be impacted. I haven't dived deep into the failures, but the tests which are failing. > The work contained in this patchset achieves the following; > > Patch 1: > Refactor dpcls_lookup and the subtable for flexibility. > In particular, add a function pointer to the subtable > structure, which enables "plugging-in" a lookup function > at runtime. This enables a number of optimizations in future. > > Patch 2 & 3: > With the function pointer in place, we refactor the existing > dpcls_lookup matching code into its own function, and later its > own file. To split it to its own file requires making various > dpcls data-structures available in the dpif-netdev.h header. > > Patch 4: > Re-implement and optimize dpcls_rule_matches_key() by removing > the "loopy-branch-ness" of the FOR_EACH() macros used. Instead > a popcount() approach is used, which is much more CPU performance > friendly, due to reduced branches/loads-stores and total work done. > > Performance: > Patches 1, 2 and 3 are performance neutral in testing here. The > fourth patch provides a significant performance improvement when > dpcls or SMC are processing packets. > > > Feedback, reviews, performance numbers weclomed! -Harry > > > Harry van Haaren (4): > dpif-netdev: implement function pointers/subtable > dpif-netdev: move dpcls lookup structures to .h > dpif-netdev: split out generic lookup function > dpif-netdev: optimized dpcls_rule_matches_key() > > lib/automake.mk | 1 + > lib/dpif-netdev-lookup-generic.c | 95 ++ > lib/dpif-netdev.c| 124 +++-- > lib/dpif-netdev.h| 130 +++ > 4 files changed, 238 insertions(+), 112 deletions(-) > create mode 100644 lib/dpif-netdev-lookup-generic.c ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Operativos - Reclutamiento Eficiente
Cursos escenciales - Webinar Interactivo – Jueves 07 de Febrero Reeclutamiento y selección eficiente de puestos operativos La dificultad de reclutar puestos operativos puede encontrar excelentes aliados en las redes sociales, las plataformas profesionales, los blogs y el internet. Nuestro webinar te ofrece analizar el panorama completo del reclutamiento en la actualidad y como sacarle provecho para reclutar específicamente puestos operativo. El participante podrá aplicar estrategias de reclutamiento y selección de personal enfocadas en cubrir las vacantes para puestos operativos. Ejes Temáticos: • Panorama actual del reclutamiento. • Perfil de los aspirantes a puestos operativos. • Herramientas de reclutamiento. • Herramientas de selección. Para mayor información, responder sobre este correo con la palabra Operativos + los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: Llamanos al (045) 55 1554 6630 www.Innovalearn.mx ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] lib/tc: Support optional tunnel id
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.
lib/netdev-tc-offloads.c | 13 +++--
lib/tc.c | 21 +++--
lib/tc.h | 1 +
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 73ce7b9..abfbaeb 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -574,7 +574,9 @@ parse_tc_flower_to_match(struct tc_flower *flower,
}
if (flower->tunnel) {
-match_set_tun_id(match, flower->key.tunnel.id);
+if (flower->mask.tunnel.id == OVS_BE64_MAX) {
+match_set_tun_id(match, flower->key.tunnel.id);
+}
if (flower->key.tunnel.ipv4.ipv4_dst) {
match_set_tun_src(match, flower->key.tunnel.ipv4.ipv4_src);
match_set_tun_dst(match, flower->key.tunnel.ipv4.ipv4_dst);
@@ -628,7 +630,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
size_t tunnel_offset =
nl_msg_start_nested(buf, OVS_KEY_ATTR_TUNNEL);
-nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID, action->encap.id);
+if (action->encap.id_present) {
+nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID,
+action->encap.id);
+}
if (action->encap.ipv4.ipv4_src) {
nl_msg_put_be32(buf, OVS_TUNNEL_KEY_ATTR_IPV4_SRC,
action->encap.ipv4.ipv4_src);
@@ -830,11 +835,13 @@ parse_put_flow_set_action(struct tc_flower *flower,
struct tc_action *action,
tunnel_len = nl_attr_get_size(set);
action->type = TC_ACT_ENCAP;
+action->encap.id_present = false;
flower->action_count++;
NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
switch (nl_attr_type(tun_attr)) {
case OVS_TUNNEL_KEY_ATTR_ID: {
action->encap.id = nl_attr_get_be64(tun_attr);
+action->encap.id_present = true;
}
break;
case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: {
@@ -1099,6 +1106,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
*match,
flower.key.tunnel.tp_dst = tnl->tp_dst;
flower.mask.tunnel.tos = tnl_mask->ip_tos;
flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
+flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
+ tnl_mask->tun_id : 0;
flower_match_to_tun_opt(&flower, tnl, tnl_mask);
flower.tunnel = true;
}
diff --git a/lib/tc.c b/lib/tc.c
index b19f075..e435663 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -571,6 +571,7 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct
tc_flower *flower)
ovs_be32 id = nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_KEY_ID]);
flower->key.tunnel.id = be32_to_be64(id);
+flower->mask.tunnel.id = OVS_BE64_MAX;
}
if (attrs[TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK]) {
flower->key.tunnel.ipv4.ipv4_src =
@@ -1014,6 +1015,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct
tc_flower *flower)
action->encap.ipv6.ipv6_dst = nl_attr_get_in6_addr(ipv6_dst);
}
action->encap.id = id ? be32_to_be64(nl_attr_get_be32(id)) : 0;
+action->encap.id_present = id ? true : false;
action->encap.tp_dst = dst_port ? nl_attr_get_be16(dst_port) : 0;
action->encap.tos = tos ? nl_attr_get_u8(tos) : 0;
action->encap.ttl = ttl ? nl_attr_get_u8(ttl) : 0;
@@ -1631,9 +1633,9 @@ nl_msg_put_act_tunnel_geneve_option(struct ofpbuf
*request,
}
static void
-nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, ovs_be64 id,
- ovs_be32 ipv4_src, ovs_be32 ipv4_dst,
- struct in6_addr *ipv6_src,
+nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
+ ovs_be64 id, ovs_be32 ipv4_src,
+ ovs_be32 ipv4_dst, struct in6_addr *ipv6_src,
struct in6_addr *ipv6_dst,
ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
struct tun_metadata tun_metadata,
@@ -1650,7 +1652,9 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
ovs_be64 id,
nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, &tun, sizeof tun);
ovs_be32 id32 = be64_to_be32(id);
-nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
+if (i
Re: [ovs-dev] [PATCH v5] ovn: Support a new Logical_Switch_Port.type - 'external'
On Mon, Jan 21, 2019 at 4:02 PM Numan Siddique wrote: > > Hi Han, > > I have addressed your comments. But before posting the patch I wanted to > get an opinion > on the HA support for these external ports. > > The proposed patch doesn't support HA. If the requested chassis goes down > for some reason > it is expected that CMS would detect it and change the requested-chassis > option to other > suitable chassis. > > The openstack OVN folks think this would be too much for the CMS to handle > and it would > complicate the code in networking-ovn which I agree with. > > Not only the complexity part. If we implement this from the CMS, then every CMS using ovn will need to replicate that behaviour. That's in my opinion a good reason why it's better to handle HA within OVN itself. > I am thinking to add the HA support on the lines of gateway chassis > support and I want to > submit this patch after adding the HA support. I think this would be > better as we won't add > more options in OVN (first requested-chassis for external ports and then > later HA chassis support). > Thoughts? > > Thanks > Numan > > > On Sat, Jan 19, 2019 at 12:42 AM Numan Siddique > wrote: > >> >> >> On Sat, Jan 19, 2019, 12:32 AM Han Zhou > >>> On Fri, Jan 18, 2019 at 10:16 AM Numan Siddique >>> wrote: >>> > >>> > >>> > >>> > On Fri, Jan 18, 2019 at 2:11 AM Han Zhou wrote: >>> >> >>> >> On Thu, Jan 17, 2019 at 11:32 AM Han Zhou wrote: >>> >> > >>> >> > On Thu, Jan 17, 2019 at 11:25 AM Numan Siddique < >>> nusid...@redhat.com> wrote: >>> >> > > >>> >> > > >>> >> > > >>> >> > > On Fri, Jan 18, 2019 at 12:21 AM Han Zhou >>> wrote: >>> >> > >> >>> >> > >> Hi Numan, >>> >> > >> >>> >> > >> With v5 the new test case "external logical port" fails. >>> >> > >> And please see more comments inlined. >>> >> > >> >>> >> > >> On Tue, Jan 15, 2019 at 12:09 PM wrote: >>> >> > >> > >>> >> > >> > From: Numan Siddique >>> >> > >> > >>> >> > >> > In the case of OpenStack + OVN, when the VMs are booted on >>> >> > >> > hypervisors supporting SR-IOV nics, there are no OVS ports >>> >> > >> > for these VMs. When these VMs sends DHCPv4, DHPCv6 or IPv6 >>> >> > >> > Router Solicitation requests, the local ovn-controller >>> >> > >> > cannot reply to these packets. OpenStack Neutron dhcp agent >>> >> > >> > service needs to be run to serve these requests. >>> >> > >> > >>> >> > >> > With the new logical port type - 'external', OVN itself can >>> >> > >> > handle these requests avoiding the need to deploy any >>> >> > >> > external services like neutron dhcp agent. >>> >> > >> > >>> >> > >> > To make use of this feature, CMS has to >>> >> > >> > - create a logical port for such VMs >>> >> > >> > - set the type to 'external' >>> >> > >> > - set requested-chassis="" in the options >>> >> > >> >column. >>> >> > >> > - create a localnet port for the logical switch >>> >> > >> > - configure the ovn-bridge-mappings option in the OVS db. >>> >> > >> > >>> >> > >> > When the ovn-controller running in that 'chassis', detects >>> >> > >> > the Port_Binding row, it adds the necessary DHCPv4/v6 OF >>> >> > >> > flows. Since the packet enters the logical switch pipeline >>> >> > >> > via the localnet port, the inport register (reg14) is set >>> >> > >> > to the tunnel key of localnet port in the match conditions. >>> >> > >> > >>> >> > >> > In case the chassis goes down for some reason, it is the >>> >> > >> > responsibility of CMS to change the 'requested-chassis' >>> >> > >> > option to some other active chassis, so that it can serve >>> >> > >> > these requests. >>> >> > >> > >>> >> > >> > When the VM with the external port, sends an ARP request for >>> >> > >> > the router ips, only the chassis which has claimed the port, >>> >> > >> > will reply to the ARP requests. Rest of the chassis on >>> >> > >> > receiving these packets drop them in the ingress switch >>> >> > >> > datapath stage - S_SWITCH_IN_EXTERNAL_PORT which is just >>> >> > >> > before S_SWITCH_IN_L2_LKUP. >>> >> > >> > >>> >> > >> > This would guarantee that only the chassis which has claimed >>> >> > >> > the external ports will run the router datapath pipeline. >>> >> > >> > >>> >> > >> > Signed-off-by: Numan Siddique >>> >> > >> > --- >>> >> > >> > >>> >> > >> > v4 -> v5 >>> >> > >> > -- >>> >> > >> > * Addressed review comments from Han Zhou. >>> >> > >> > >>> >> > >> > v3 -> v4 >>> >> > >> > -- >>> >> > >> > * Updated the documention as per Han Zhou's suggestion. >>> >> > >> > >>> >> > >> > v2 -> v3 >>> >> > >> > --- >>> >> > >> > * Rebased >>> >> > >> > >>> >> > >> > ovn/controller/binding.c| 12 + >>> >> > >> > ovn/controller/lflow.c | 41 ++- >>> >> > >> > ovn/controller/lflow.h | 2 + >>> >> > >> > ovn/controller/lport.c | 26 ++ >>> >> > >> > ovn/controller/lport.h | 5 + >>> >> > >> > ovn/controller/ovn-controller.c | 6 + >>> >> > >> > ovn/lib/ovn-util.c | 1 + >>> >> > >> > ovn/northd/ovn-no
Re: [ovs-dev] [PATCH v5] ovn: Support a new Logical_Switch_Port.type - 'external'
Hi Han, I have addressed your comments. But before posting the patch I wanted to get an opinion on the HA support for these external ports. The proposed patch doesn't support HA. If the requested chassis goes down for some reason it is expected that CMS would detect it and change the requested-chassis option to other suitable chassis. The openstack OVN folks think this would be too much for the CMS to handle and it would complicate the code in networking-ovn which I agree with. I am thinking to add the HA support on the lines of gateway chassis support and I want to submit this patch after adding the HA support. I think this would be better as we won't add more options in OVN (first requested-chassis for external ports and then later HA chassis support). Thoughts? Thanks Numan On Sat, Jan 19, 2019 at 12:42 AM Numan Siddique wrote: > > > On Sat, Jan 19, 2019, 12:32 AM Han Zhou >> On Fri, Jan 18, 2019 at 10:16 AM Numan Siddique >> wrote: >> > >> > >> > >> > On Fri, Jan 18, 2019 at 2:11 AM Han Zhou wrote: >> >> >> >> On Thu, Jan 17, 2019 at 11:32 AM Han Zhou wrote: >> >> > >> >> > On Thu, Jan 17, 2019 at 11:25 AM Numan Siddique >> wrote: >> >> > > >> >> > > >> >> > > >> >> > > On Fri, Jan 18, 2019 at 12:21 AM Han Zhou >> wrote: >> >> > >> >> >> > >> Hi Numan, >> >> > >> >> >> > >> With v5 the new test case "external logical port" fails. >> >> > >> And please see more comments inlined. >> >> > >> >> >> > >> On Tue, Jan 15, 2019 at 12:09 PM wrote: >> >> > >> > >> >> > >> > From: Numan Siddique >> >> > >> > >> >> > >> > In the case of OpenStack + OVN, when the VMs are booted on >> >> > >> > hypervisors supporting SR-IOV nics, there are no OVS ports >> >> > >> > for these VMs. When these VMs sends DHCPv4, DHPCv6 or IPv6 >> >> > >> > Router Solicitation requests, the local ovn-controller >> >> > >> > cannot reply to these packets. OpenStack Neutron dhcp agent >> >> > >> > service needs to be run to serve these requests. >> >> > >> > >> >> > >> > With the new logical port type - 'external', OVN itself can >> >> > >> > handle these requests avoiding the need to deploy any >> >> > >> > external services like neutron dhcp agent. >> >> > >> > >> >> > >> > To make use of this feature, CMS has to >> >> > >> > - create a logical port for such VMs >> >> > >> > - set the type to 'external' >> >> > >> > - set requested-chassis="" in the options >> >> > >> >column. >> >> > >> > - create a localnet port for the logical switch >> >> > >> > - configure the ovn-bridge-mappings option in the OVS db. >> >> > >> > >> >> > >> > When the ovn-controller running in that 'chassis', detects >> >> > >> > the Port_Binding row, it adds the necessary DHCPv4/v6 OF >> >> > >> > flows. Since the packet enters the logical switch pipeline >> >> > >> > via the localnet port, the inport register (reg14) is set >> >> > >> > to the tunnel key of localnet port in the match conditions. >> >> > >> > >> >> > >> > In case the chassis goes down for some reason, it is the >> >> > >> > responsibility of CMS to change the 'requested-chassis' >> >> > >> > option to some other active chassis, so that it can serve >> >> > >> > these requests. >> >> > >> > >> >> > >> > When the VM with the external port, sends an ARP request for >> >> > >> > the router ips, only the chassis which has claimed the port, >> >> > >> > will reply to the ARP requests. Rest of the chassis on >> >> > >> > receiving these packets drop them in the ingress switch >> >> > >> > datapath stage - S_SWITCH_IN_EXTERNAL_PORT which is just >> >> > >> > before S_SWITCH_IN_L2_LKUP. >> >> > >> > >> >> > >> > This would guarantee that only the chassis which has claimed >> >> > >> > the external ports will run the router datapath pipeline. >> >> > >> > >> >> > >> > Signed-off-by: Numan Siddique >> >> > >> > --- >> >> > >> > >> >> > >> > v4 -> v5 >> >> > >> > -- >> >> > >> > * Addressed review comments from Han Zhou. >> >> > >> > >> >> > >> > v3 -> v4 >> >> > >> > -- >> >> > >> > * Updated the documention as per Han Zhou's suggestion. >> >> > >> > >> >> > >> > v2 -> v3 >> >> > >> > --- >> >> > >> > * Rebased >> >> > >> > >> >> > >> > ovn/controller/binding.c| 12 + >> >> > >> > ovn/controller/lflow.c | 41 ++- >> >> > >> > ovn/controller/lflow.h | 2 + >> >> > >> > ovn/controller/lport.c | 26 ++ >> >> > >> > ovn/controller/lport.h | 5 + >> >> > >> > ovn/controller/ovn-controller.c | 6 + >> >> > >> > ovn/lib/ovn-util.c | 1 + >> >> > >> > ovn/northd/ovn-northd.8.xml | 37 ++- >> >> > >> > ovn/northd/ovn-northd.c | 85 - >> >> > >> > ovn/ovn-architecture.7.xml | 78 + >> >> > >> > ovn/ovn-nb.xml | 47 +++ >> >> > >> > tests/ovn.at| 530 >> +++- >> >> > >> > 12 files changed, 848 insertions(+), 22 deletions(-) >> >> > >> > >> >> > >> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c >
Re: [ovs-dev] dpif-netdev: optimized dpcls_rule_matches_key()
Bleep bloop. Greetings Harry van Haaren, 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: WARNING: Line is 94 characters long (recommended limit is 79) #131 FILE: lib/dpif-netdev.h:163: fail |= dpcls_verify_unit(rle_u0, pkt_u0, &rle_blocks[0], &msk_blocks[0], &pkt_blocks[0]); WARNING: Line is 95 characters long (recommended limit is 79) #132 FILE: lib/dpif-netdev.h:164: fail |= dpcls_verify_unit(rle_u1, pkt_u1, &rle_blocks[rle_u0_pop], &msk_blocks[rle_u0_pop], Lines checked: 144, Warnings: 2, Errors: 0 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] lib/tc: Support optional tunnel id
On Mon, Jan 21, 2019 at 11:53:20AM +, Adi Nissim wrote: > Hi Simon, > > thank you for the comments > > Is the tnl->flags check necessary? What is the meaning of a match on the > flags if the tunnel type doesn't support that field. Is it possible to > configure such a flow? > > > it is possible to configure such flow, for example GRE tunnel without key is > a possible option and in this case the key flag will not be set. > > this is why the check tnl->flags is needed. Thanks, that makes sense. > > It seems to me that the code without your patch assumes that the mask is > all-ones, or in other words an exact-match. This is the only match currently > supported by TC flower. > > So I think that checking the mask.tunnel.id is a good idea, but shouldn't the > check be such that OVS: > > 1. Offloads matches that include an exact match on the tunnel id 2. Skips > matching the tunnel id is the mask is 0 (the purpose of this patch) 3. > Rejects offloading flows where the tunnel id mask is neither 0 nor >all-ones > > I will make the necessary changes and send a new patch. Thanks, much appreciated. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] dpif-netdev: split out generic lookup function
Bleep bloop. Greetings Harry van Haaren, 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: Improper whitespace around control block
#77 FILE: lib/dpif-netdev-lookup-generic.c:41:
NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(value, key, mask->mf.map) {
ERROR: C99 style comment
#119 FILE: lib/dpif-netdev-lookup-generic.c:83:
//lookups_match += subtable_pos;
ERROR: Improper whitespace around control block
#276 FILE: lib/dpif-netdev.h:100:
#define NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(VALUE, KEY, FLOWMAP) \
ERROR: Improper whitespace around control block
#277 FILE: lib/dpif-netdev.h:101:
MINIFLOW_FOR_EACH_IN_FLOWMAP(VALUE, &(KEY)->mf, FLOWMAP)
Lines checked: 289, Warnings: 0, Errors: 4
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] dpif-netdev: implement function pointers/subtable
Bleep bloop. Greetings Harry van Haaren, 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: C99 style comment #110 FILE: lib/dpif-netdev.c:7805: //lookups_match += subtable_pos; Lines checked: 203, Warnings: 0, 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
[ovs-dev] [PATCH v4 3/4] dpif-netdev: split out generic lookup function
This commit splits the generic hash-lookup-verify
function to its own file. In doing so, we must move
some MACRO definitions to dpif-netdev.h
Signed-off-by: Harry van Haaren
---
lib/automake.mk | 1 +
lib/dpif-netdev-lookup-generic.c | 95
lib/dpif-netdev.c| 81 +--
lib/dpif-netdev.h| 16 ++
4 files changed, 113 insertions(+), 80 deletions(-)
create mode 100644 lib/dpif-netdev-lookup-generic.c
diff --git a/lib/automake.mk b/lib/automake.mk
index b1ff495ff..b29128ac4 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -78,6 +78,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dp-packet.h \
lib/dp-packet.c \
lib/dpdk.h \
+ lib/dpif-netdev-lookup-generic.c \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
lib/dpif-netdev-perf.c \
diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-generic.c
new file mode 100644
index 0..8ae8ff59d
--- /dev/null
+++ b/lib/dpif-netdev-lookup-generic.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2016, 2017 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "dpif-netdev.h"
+
+#include "bitmap.h"
+#include "cmap.h"
+
+#include "dp-packet.h"
+#include "dpif.h"
+#include "dpif-netdev-perf.h"
+#include "dpif-provider.h"
+#include "flow.h"
+#include "packets.h"
+#include "pvector.h"
+
+/* Returns a hash value for the bits of 'key' where there are 1-bits in
+ * 'mask'. */
+static inline uint32_t
+netdev_flow_key_hash_in_mask(const struct netdev_flow_key *key,
+ const struct netdev_flow_key *mask)
+{
+const uint64_t *p = miniflow_get_values(&mask->mf);
+uint32_t hash = 0;
+uint64_t value;
+
+NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(value, key, mask->mf.map) {
+hash = hash_add64(hash, value & *p);
+p++;
+}
+
+return hash_finish(hash, (p - miniflow_get_values(&mask->mf)) * 8);
+}
+
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+ uint32_t keys_map,
+ const struct netdev_flow_key *keys[],
+ struct dpcls_rule **rules)
+{
+int i;
+/* Compute hashes for the remaining keys. Each search-key is
+ * masked with the subtable's mask to avoid hashing the wildcarded
+ * bits. */
+uint32_t hashes[NETDEV_MAX_BURST];
+ULLONG_FOR_EACH_1(i, keys_map) {
+hashes[i] = netdev_flow_key_hash_in_mask(keys[i],
+ &subtable->mask);
+}
+
+/* Lookup. */
+const struct cmap_node *nodes[NETDEV_MAX_BURST];
+uint32_t found_map =
+cmap_find_batch(&subtable->rules, keys_map, hashes, nodes);
+/* Check results. When the i-th bit of found_map is set, it means
+ * that a set of nodes with a matching hash value was found for the
+ * i-th search-key. Due to possible hash collisions we need to check
+ * which of the found rules, if any, really matches our masked
+ * search-key. */
+ULLONG_FOR_EACH_1(i, found_map) {
+struct dpcls_rule *rule;
+
+CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
+if (OVS_LIKELY(dpcls_rule_matches_key(rule, keys[i]))) {
+rules[i] = rule;
+/* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
+ * within one second optimization interval. */
+subtable->hit_cnt++;
+//lookups_match += subtable_pos;
+goto next;
+}
+}
+/* None of the found rules was a match. Reset the i-th bit to
+ * keep searching this key in the next subtable. */
+ULLONG_SET0(found_map, i); /* Did not match. */
+next:
+; /* Keep Sparse happy. */
+}
+
+return found_map;
+}
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 39ea16531..bcf77db22 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -233,14 +233,6 @@ struct dpcls {
struct pvector subtables;
};
-/* A rule to be inserted to the classifier. */
-struct dpcls_rule {
-struct cmap_node cmap_node; /* Within struct dpcls_subtable 'rules'. */
-st
[ovs-dev] [PATCH v4 4/4] dpif-netdev: optimized dpcls_rule_matches_key()
This commit optimizes the dpcls_rule_matches_key() function
by refactoring the code to be shorter and less branchy. The
main code-change (and optimization) is to use popcount and
mask to calculate the packet block index.
This code is split out to the header file, which is marked as
static inline. The code itself is refactored into two functions,
one to iterate the "units" of the miniflow, the other to compare
the actual miniflow "blocks" and compare them with the mask/value.
The verify_unit() function is called twice, and should/will be inlined
by the compiler in optimized builds. As a result the while() loop
inside is hence present in two locations in the caller function.
This gives each while() loop its own entry in the branch-predictor,
helping it to correctly predict each units loop iterations.
Finally, we always compute all blocks, and then return match or
not-matching in a branch-free manner. This allows the pipeline to
execute better, by removing non-predictable branches.
Signed-off-by: Harry van Haaren
---
This patch optimizes rule_verify() signficantly, please benchmark
before and after this patch and see what gains your workload sees.
v4:
- Fixed bug in popcount offset calculation, thanks Ilya for reporting
- Comments cleaned up and reworded for clarity
v3:
- Fix "last minute variable rename fix" which broke the build :/
---
lib/dpif-netdev.c | 18 -
lib/dpif-netdev.h | 68 +--
2 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bcf77db22..bee85a8f3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7669,24 +7669,6 @@ dpcls_remove(struct dpcls *cls, struct dpcls_rule *rule)
}
}
-/* Returns true if 'target' satisfies 'key' in 'mask', that is, if each 1-bit
- * in 'mask' the values in 'key' and 'target' are the same. */
-bool
-dpcls_rule_matches_key(const struct dpcls_rule *rule,
- const struct netdev_flow_key *target)
-{
-const uint64_t *keyp = miniflow_get_values(&rule->flow.mf);
-const uint64_t *maskp = miniflow_get_values(&rule->mask->mf);
-uint64_t value;
-
-NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(value, target, rule->flow.mf.map) {
-if (OVS_UNLIKELY((value & *maskp++) != *keyp++)) {
-return false;
-}
-}
-return true;
-}
-
/* For each miniflow in 'keys' performs a classifier lookup writing the result
* into the corresponding slot in 'rules'. If a particular entry in 'keys' is
* NULL it is skipped.
diff --git a/lib/dpif-netdev.h b/lib/dpif-netdev.h
index 7195b9791..13017ce14 100644
--- a/lib/dpif-netdev.h
+++ b/lib/dpif-netdev.h
@@ -100,9 +100,73 @@ struct dpcls_subtable {
#define NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(VALUE, KEY, FLOWMAP) \
MINIFLOW_FOR_EACH_IN_FLOWMAP(VALUE, &(KEY)->mf, FLOWMAP)
-bool dpcls_rule_matches_key(const struct dpcls_rule *rule,
-const struct netdev_flow_key *target);
+/* Iterate all bits set in the *rle_unit*, lookup the block of metadata based
+ * on the packet miniflow, and compare it for "matching" the rule, using the
+ * subtable mask in the process. Note that the pointers passed in to this
+ * function are already adjusted for the unit offset. */
+static inline int32_t
+dpcls_verify_unit(const uint64_t rle_unit, const uint64_t pkt_unit,
+ const uint64_t *rle, const uint64_t *msk,
+ const uint64_t *pkt)
+{
+int match_fail = 0;
+int linear_idx = 0;
+
+uint64_t iter = rle_unit;
+while (iter) {
+uint64_t low_bit = iter & (-iter);
+iter &= ~(low_bit);
+
+uint64_t low_mask = low_bit - 1;
+uint64_t bits = (low_mask & pkt_unit);
+uint64_t blk_idx = __builtin_popcountll(bits);
+
+/* Take packet, mask bits away, compare against rule.
+ * Results in 1 for matching, so ! to invert to fail */
+match_fail |= !((pkt[blk_idx] & msk[linear_idx]) == rle[linear_idx]);
+linear_idx++;
+}
+
+return match_fail;
+}
+/* match rule and target (aka packet), to understand if the rule applies to
+ * this packet. The actual miniflow-unit iteration is performed in
+ * the *dpcls_verify_unit* function, this just wraps the two unit calls */
+static inline int
+dpcls_rule_matches_key(const struct dpcls_rule *rule,
+ const struct netdev_flow_key *target)
+{
+/* retrieve the "block" pointers for the packet, rule and subtable mask */
+const uint64_t *rle_blocks = miniflow_get_values(&rule->flow.mf);
+const uint64_t *msk_blocks = miniflow_get_values(&rule->mask->mf);
+const uint64_t *pkt_blocks = miniflow_get_values(&target->mf);
+
+/* fetch the rule bits to iterate */
+const uint64_t rle_u0 = rule->flow.mf.map.bits[0];
+const uint64_t rle_u1 = rule->flow.mf.map.bits[1];
+
+/* fetch the packet bits to navigate the packet's miniflow block indexes */
+const uint64_t pk
[ovs-dev] [PATCH v4 2/4] dpif-netdev: move dpcls lookup structures to .h
This commit moves some data-structures to be available
in the dpif-netdev.h header. This allows specific implementations
of the subtable lookup function to include just that header
file, and not require that the code exists in dpif-netdev.c
Signed-off-by: Harry van Haaren
---
lib/dpif-netdev.c | 50 ---
lib/dpif-netdev.h | 50 +++
2 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3d4e3f737..39ea16531 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -127,15 +127,6 @@ static struct odp_support dp_netdev_support = {
.ct_orig_tuple6 = true,
};
-/* Stores a miniflow with inline values */
-
-struct netdev_flow_key {
-uint32_t hash; /* Hash function differs for different users. */
-uint32_t len;/* Length of the following miniflow (incl. map). */
-struct miniflow mf;
-uint64_t buf[FLOW_MAX_PACKET_U64S];
-};
-
/* EMC cache and SMC cache compose the datapath flow cache (DFC)
*
* Exact match cache for frequently used flows
@@ -7514,47 +7505,6 @@ dpif_dummy_register(enum dummy_level level)
/* Datapath Classifier. */
-/* forward declaration for lookup_func typedef */
-struct dpcls_subtable;
-
-/** Lookup function for a subtable in the dpcls. This function is called
- * by each subtable with an array of packets, and a bitmask of packets to
- * perform the lookup on. Using a function pointer gives flexibility to
- * optimize the lookup function based on subtable properties and the
- * CPU instruction set available at runtime.
- */
-typedef uint32_t (*dpcls_subtable_lookup_func)(struct dpcls_subtable *subtable,
-uint32_t keys_map, const struct netdev_flow_key *keys[],
-struct dpcls_rule **rules);
-
-/** Prototype for generic lookup func, using same code path as before */
-uint32_t
-dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
- uint32_t keys_map,
- const struct netdev_flow_key *keys[],
- struct dpcls_rule **rules);
-
-
-/* A set of rules that all have the same fields wildcarded. */
-struct dpcls_subtable {
-/* The fields are only used by writers. */
-struct cmap_node cmap_node OVS_GUARDED; /* Within dpcls 'subtables_map'. */
-
-/* These fields are accessed by readers. */
-struct cmap rules; /* Contains "struct dpcls_rule"s. */
-uint32_t hit_cnt;/* Number of match hits in subtable in current
-optimization interval. */
-
-/* the lookup function to use for this subtable. If there is a known
- * property of the subtable (eg: only 3 bits of miniflow metadata is
- * used for the lookup) then this can point at an optimized version of
- * the lookup function for this particular subtable. */
-dpcls_subtable_lookup_func lookup_func;
-
-struct netdev_flow_key mask; /* Wildcards for fields (const). */
-/* 'mask' must be the last field, additional space is allocated here. */
-};
-
/* Initializes 'cls' as a classifier that initially contains no classification
* rules. */
static void
diff --git a/lib/dpif-netdev.h b/lib/dpif-netdev.h
index 6db6ed2e2..720425fb3 100644
--- a/lib/dpif-netdev.h
+++ b/lib/dpif-netdev.h
@@ -24,6 +24,7 @@
#include "openvswitch/types.h"
#include "dp-packet.h"
#include "packets.h"
+#include "cmap.h"
#ifdef __cplusplus
extern "C" {
@@ -38,6 +39,55 @@ bool dpif_is_netdev(const struct dpif *);
#define NR_QUEUE 1
#define NR_PMD_THREADS 1
+/* forward declaration for lookup_func typedef */
+struct dpcls_subtable;
+struct dpcls_rule;
+
+/* must be public as it is intantiated in subtable struct below */
+struct netdev_flow_key {
+uint32_t hash; /* Hash function differs for different users. */
+uint32_t len;/* Length of the following miniflow (incl. map). */
+struct miniflow mf;
+uint64_t buf[FLOW_MAX_PACKET_U64S];
+};
+
+/** Lookup function for a subtable in the dpcls. This function is called
+ * by each subtable with an array of packets, and a bitmask of packets to
+ * perform the lookup on. Using a function pointer gives flexibility to
+ * optimize the lookup function based on subtable properties and the
+ * CPU instruction set available at runtime.
+ */
+typedef uint32_t (*dpcls_subtable_lookup_func)(struct dpcls_subtable *subtable,
+uint32_t keys_map, const struct netdev_flow_key *keys[],
+struct dpcls_rule **rules);
+
+/** Prototype for generic lookup func, using same code path as before */
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+ uint32_t keys_map,
+ const struct netdev_flow_key *keys[],
+ struct dpcls_rule **rules);
+
+/* A set of rules that all have the same fields wildca
[ovs-dev] [PATCH v4 1/4] dpif-netdev: implement function pointers/subtable
This allows plugging-in of different subtable hash-lookup-verify
routines, and allows special casing of those functions based on
known context (eg: # of bits set) of the specific subtable.
Signed-off-by: Harry van Haaren
---
lib/dpif-netdev.c | 127 +++---
1 file changed, 87 insertions(+), 40 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0f57e3f8a..3d4e3f737 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7514,6 +7514,27 @@ dpif_dummy_register(enum dummy_level level)
/* Datapath Classifier. */
+/* forward declaration for lookup_func typedef */
+struct dpcls_subtable;
+
+/** Lookup function for a subtable in the dpcls. This function is called
+ * by each subtable with an array of packets, and a bitmask of packets to
+ * perform the lookup on. Using a function pointer gives flexibility to
+ * optimize the lookup function based on subtable properties and the
+ * CPU instruction set available at runtime.
+ */
+typedef uint32_t (*dpcls_subtable_lookup_func)(struct dpcls_subtable *subtable,
+uint32_t keys_map, const struct netdev_flow_key *keys[],
+struct dpcls_rule **rules);
+
+/** Prototype for generic lookup func, using same code path as before */
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+ uint32_t keys_map,
+ const struct netdev_flow_key *keys[],
+ struct dpcls_rule **rules);
+
+
/* A set of rules that all have the same fields wildcarded. */
struct dpcls_subtable {
/* The fields are only used by writers. */
@@ -7523,6 +7544,13 @@ struct dpcls_subtable {
struct cmap rules; /* Contains "struct dpcls_rule"s. */
uint32_t hit_cnt;/* Number of match hits in subtable in current
optimization interval. */
+
+/* the lookup function to use for this subtable. If there is a known
+ * property of the subtable (eg: only 3 bits of miniflow metadata is
+ * used for the lookup) then this can point at an optimized version of
+ * the lookup function for this particular subtable. */
+dpcls_subtable_lookup_func lookup_func;
+
struct netdev_flow_key mask; /* Wildcards for fields (const). */
/* 'mask' must be the last field, additional space is allocated here. */
};
@@ -7576,6 +7604,10 @@ dpcls_create_subtable(struct dpcls *cls, const struct
netdev_flow_key *mask)
cmap_init(&subtable->rules);
subtable->hit_cnt = 0;
netdev_flow_key_clone(&subtable->mask, mask);
+
+/* decide which hash/lookup/verify function to use */
+subtable->lookup_func = dpcls_subtable_lookup_generic;
+
cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash);
/* Add the new subtable at the end of the pvector (with no hits yet) */
pvector_insert(&cls->subtables, subtable, 0);
@@ -7736,6 +7768,54 @@ dpcls_rule_matches_key(const struct dpcls_rule *rule,
return true;
}
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+ uint32_t keys_map,
+ const struct netdev_flow_key *keys[],
+ struct dpcls_rule **rules)
+{
+int i;
+/* Compute hashes for the remaining keys. Each search-key is
+ * masked with the subtable's mask to avoid hashing the wildcarded
+ * bits. */
+uint32_t hashes[NETDEV_MAX_BURST];
+ULLONG_FOR_EACH_1(i, keys_map) {
+hashes[i] = netdev_flow_key_hash_in_mask(keys[i],
+ &subtable->mask);
+}
+
+/* Lookup. */
+const struct cmap_node *nodes[NETDEV_MAX_BURST];
+uint32_t found_map =
+cmap_find_batch(&subtable->rules, keys_map, hashes, nodes);
+/* Check results. When the i-th bit of found_map is set, it means
+ * that a set of nodes with a matching hash value was found for the
+ * i-th search-key. Due to possible hash collisions we need to check
+ * which of the found rules, if any, really matches our masked
+ * search-key. */
+ULLONG_FOR_EACH_1(i, found_map) {
+struct dpcls_rule *rule;
+
+CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
+if (OVS_LIKELY(dpcls_rule_matches_key(rule, keys[i]))) {
+rules[i] = rule;
+/* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
+ * within one second optimization interval. */
+subtable->hit_cnt++;
+//lookups_match += subtable_pos;
+goto next;
+}
+}
+/* None of the found rules was a match. Reset the i-th bit to
+ * keep searching this key in the next subtable. */
+ULLONG_SET0(found_map, i); /* Did no
[ovs-dev] [PATCH v4 0/4] dpcls subtable miniflow optimizations
Hi Folks, This patchset is a v4, changes from v3 are a fix to the issue reported by Ilya (see v3 patchset for details). Note that this patchset enables the work as presented at OVS Conf last December, particularly this is the function pointer part: https://www.youtube.com/watch?v=5-MDlpUIOBE The work contained in this patchset achieves the following; Patch 1: Refactor dpcls_lookup and the subtable for flexibility. In particular, add a function pointer to the subtable structure, which enables "plugging-in" a lookup function at runtime. This enables a number of optimizations in future. Patch 2 & 3: With the function pointer in place, we refactor the existing dpcls_lookup matching code into its own function, and later its own file. To split it to its own file requires making various dpcls data-structures available in the dpif-netdev.h header. Patch 4: Re-implement and optimize dpcls_rule_matches_key() by removing the "loopy-branch-ness" of the FOR_EACH() macros used. Instead a popcount() approach is used, which is much more CPU performance friendly, due to reduced branches/loads-stores and total work done. Performance: Patches 1, 2 and 3 are performance neutral in testing here. The fourth patch provides a significant performance improvement when dpcls or SMC are processing packets. Feedback, reviews, performance numbers weclomed! -Harry Harry van Haaren (4): dpif-netdev: implement function pointers/subtable dpif-netdev: move dpcls lookup structures to .h dpif-netdev: split out generic lookup function dpif-netdev: optimized dpcls_rule_matches_key() lib/automake.mk | 1 + lib/dpif-netdev-lookup-generic.c | 95 ++ lib/dpif-netdev.c| 124 +++-- lib/dpif-netdev.h| 130 +++ 4 files changed, 238 insertions(+), 112 deletions(-) create mode 100644 lib/dpif-netdev-lookup-generic.c -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 0/2] Do not rewrite fields with the same values as
ping -Original Message- From: Eli Britstein Sent: Monday, January 21, 2019 2:14 PM To: Roi Dayan Cc: Simon Horman ; Jarno Rajahalme ; Ben Pfaff ; William Tu ; Andy Zhou Subject: Re: [PATCH V2 0/2] Do not rewrite fields with the same values as ping On 1/13/2019 9:27 AM, Eli Britstein wrote: > > On 1/11/2019 8:28 AM, Eli Britstein wrote: >> Hi > v2: removed unnecessary loop >> This patch set avoids unnecessary rewrite actions to fields with the >> same values as matched on. >> >> Patch 1 is a pre-step by defining ovs key structs using macros >> >> Patch 2 avoids the unnecessary rewrites and adapts the tests >> accordingly >> >> >> Travis link: >> https://travis-ci.org/roidayan/ovs/builds/477699710 >> Appvoyer link: >> https://ci.appveyor.com/project/roidayan/ovs/builds/21515073 >> >> Thanks, >> Eli >> >> >> Eli Britstein (2): >> datapath: Declare ovs key structures using macros >> odp-util: Do not rewrite fields with the same values as matched >> >> build-aux/extract-odp-netlink-h | 3 +- >> datapath/linux/compat/include/linux/openvswitch.h | 102 >> +--- >> lib/odp-util.c | 109 >> -- >> tests/mpls-xlate.at | 2 +- >> tests/ofproto-dpif.at | 14 +-- >> 5 files changed, 178 insertions(+), 52 deletions(-) >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] OK
Greetings Compliment of the day to you. I am Mr. Ahmed Zama, I am sending this brief letter to solicit your partnership to transfer €15 million Euros into your account for investment in your country. I shall send you more information and procedures when I receive positive response from you. Thanks Ahmed Zama +22675844869 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] lib/tc: Support optional tunnel id
Hi Simon, thank you for the comments Is the tnl->flags check necessary? What is the meaning of a match on the flags if the tunnel type doesn't support that field. Is it possible to configure such a flow? it is possible to configure such flow, for example GRE tunnel without key is a possible option and in this case the key flag will not be set. this is why the check tnl->flags is needed. It seems to me that the code without your patch assumes that the mask is all-ones, or in other words an exact-match. This is the only match currently supported by TC flower. So I think that checking the mask.tunnel.id is a good idea, but shouldn't the check be such that OVS: 1. Offloads matches that include an exact match on the tunnel id 2. Skips matching the tunnel id is the mask is 0 (the purpose of this patch) 3. Rejects offloading flows where the tunnel id mask is neither 0 nor all-ones I will make the necessary changes and send a new patch. Thanks, Adi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v4 0/3] fixes for ftp alg in userspace dp.
Hello Ben, On Sat, Jan 19, 2019 at 1:24 AM Ben Pfaff wrote: > On Tue, Jan 15, 2019 at 06:58:14PM -0800, Darrell Ball wrote: > > Some small fixes to make the ftp alg behave a little better. > > I tried to come up with better tests but I am not too sure if we want to > > have a dependency on yet another external tool for it (lftp). > > > > Changelog since v3: > > - applied Darrell comments > > - more child connections, little style change in patch 1 > > - added a DNAT tests with "reverse skew" in patch 2 > > - added patch from Darrell to fix seq/ack adjustments > > > > Changelog since v2: > > - applied Darrell comments > > > > Changelog since v1: > > - dropped unnecessary changes > > - squashed code changes and tests together > > - sticked closer to the original code > > Thanks for figuring this out with David. > > I applied this to master and backported to branch-2.10 and branch-2.11. > Beyond that it failed to apply cleanly, so please submit backported > patches if it needs to go farther. > I will be out this week, but I can send those patches on 2.9 next week, if this is okay with you. -- David Marchand ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] conntrack: fix ftp ipv4 address substitution
replace_substring() wants the total size of the string in order to move
the end of the string after the part being replaced.
When replacing the ipv4 address in repl_ftp_v4_addr(), the remain_size
variable must be updated after replace_substring() has been called, not
before.
Besides, the substraction is off by one, since we need to account for the
',' that is skipped.
This goes unnoticed most of the time, unless you choose carefully your
setup and ip addresses.
Example for ftp in passive mode with address 10.1.1.200 DNAT'd to
10.1.100.1:
Breakpoint 1, replace_substring (substr=0x1e68497
"10,1,100,1,241,144).\r\n", substr_size=2 '\002', total_size=20 '\024',
rep_str=0x7fff92ee4e50 "10", rep_str_size=2 '\002') at
lib/conntrack.c:2864
Breakpoint 1, replace_substring (substr=0x1e6849a
"1,100,1,241,144).\r\n", substr_size=1 '\001', total_size=19 '\023',
rep_str=0x7fff92ee4e50 "1", rep_str_size=1 '\001') at
lib/conntrack.c:2864
Breakpoint 1, replace_substring (substr=0x1e6849c "100,1,241,144).\r\n",
substr_size=3 '\003', total_size=16 '\020', rep_str=0x7fff92ee4e50 "1",
rep_str_size=1 '\001') at lib/conntrack.c:2864
Breakpoint 1, replace_substring (substr=0x1e6849e "1,241,144).\r.\r\n",
substr_size=1 '\001', total_size=15 '\017', rep_str=0x7fff92ee4e50
"200", rep_str_size=3 '\003') at lib/conntrack.c:2864
>From the "unnated" side, the payload ends up being incorrectly terminated:
0x0040: b715 3232 3720 456e 7465 7269 6e67 2050 ..227.Entering.P
0x0050: 6173 7369 7665 204d 6f64 6520 2831 302c assive.Mode.(10,
0x0060: 312c 312c 3230 302c 3234 312c 3134 3429 1,1,200,241,144)
0x0070: 2e0d 2e ...
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: David Marchand
---
lib/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f732b9e..8eddc8e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2817,7 +2817,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
v4_addr_rep,
char *next_delim = memchr(byte_str, ',', 4);
ovs_assert(next_delim);
int substr_size = next_delim - byte_str;
-remain_size -= substr_size;
/* Compose the new string for this octet, and replace it. */
char rep_str[4];
@@ -2825,6 +2824,7 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32
v4_addr_rep,
int replace_size = sprintf(rep_str, "%d", rep_byte);
replace_substring(byte_str, substr_size, remain_size,
rep_str, replace_size);
+remain_size -= substr_size + 1;
overall_delta += replace_size - substr_size;
/* Advance past the octet and the following comma. */
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
