Re: [ovs-dev] [PATCH ovn v1 1/2] tests: Updated expected log message
On Sun, Dec 8, 2019 at 5:18 AM Russell Bryant wrote: > > A previous commit added more detail to this log message. Fix the test > to reflect the new text. > > Signed-off-by: Russell Bryant Hi Russell, Thanks for fixing the failing test case. Acked-by: Dumitru Ceara > --- > tests/ovn.at | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 8f4d9a440..1d5369341 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -13618,22 +13618,22 @@ ovn-nbctl list logical_switch_port > # Now try to add duplicate addresses on a new port. These should all fail > ovn-nbctl --wait=sb lsp-add sw1 sw1-p5 > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 10.0.0.1"], > [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 10.0.0.1 > +[ovn-nbctl: Error on switch sw1: duplicate IPv4 address '10.0.0.1' found on > logical switch port 'sw1-p1' > ]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 10.0.0.2"], > [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 10.0.0.2 > +[ovn-nbctl: Error on switch sw1: duplicate IPv4 address '10.0.0.2' found on > logical switch port 'sw1-p1' > ]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 aef0::1"], > [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv6 address aef0::1 > +[ovn-nbctl: Error on switch sw1: duplicate IPv6 address 'aef0::1' found on > logical switch port 'sw1-p1' > ]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 aef0::2"], > [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv6 address aef0::2 > +[ovn-nbctl: Error on switch sw1: duplicate IPv6 address 'aef0::2' found on > logical switch port 'sw1-p1' > ]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 > 192.168.0.2"], [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 192.168.0.2 > +[ovn-nbctl: Error on switch sw1: duplicate IPv4 address '192.168.0.2' found > on logical switch port 'sw1-p2' > ]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04 > 192.168.0.3"], [1], [], > -[ovn-nbctl: Error on switch sw1: duplicate IPv4 address 192.168.0.3 > +[ovn-nbctl: Error on switch sw1: duplicate IPv4 address '192.168.0.3' found > on logical switch port 'sw1-p3' > ]) > > # Now try re-setting sw1-p1. This should succeed > -- > 2.23.0 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 08/19] netdev-offload-dpdk: Protect UFID map by mutex
On 12/9/2019 9:15 AM, 贺鹏 wrote:
> Hi,
>
>
> cmap support multiple readers and one writer.
> since all write operations are performed in the offload thread,
> why need a mutex here? Maybe I miss some patches.
Collecting statistics using netdev_flow_get in patch 10/20 is another
thread. There, it gets the rte_flow, but if not protected it might get
destroyed by the offload thread before it uses it.
>
>
>
> Eli Britstein 于2019年12月8日周日 下午9:23写道：
>
>> Flow deletion and dumping for statistics collection are called from
>> different threads. As a pre-step towards collecting HW statistics,
>> protect the UFID map by mutex to make it thread safe.
>>
>> Signed-off-by: Eli Britstein
>> ---
>> lib/netdev-offload-dpdk.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index b2ec05cec..5568400b6 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -51,6 +51,7 @@ static struct vlog_rate_limit error_rl =
>> VLOG_RATE_LIMIT_INIT(100, 5);
>>* A mapping from ufid to dpdk rte_flow.
>>*/
>> static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
>> +static struct ovs_mutex ufid_map_mutex = OVS_MUTEX_INITIALIZER;
>>
>> struct ufid_to_rte_flow_data {
>> struct cmap_node node;
>> @@ -630,8 +631,11 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
>>struct rte_flow *rte_flow)
>> {
>> struct rte_flow_error error;
>> -int ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
>> +int ret;
>> +
>> +ovs_mutex_lock(_map_mutex);
>>
>> +ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
>> if (ret == 0) {
>> ufid_to_rte_flow_disassociate(ufid);
>> VLOG_DBG("%s: removed rte flow %p associated with ufid " UUID_FMT
>> "\n",
>> @@ -642,6 +646,7 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
>>netdev_get_name(netdev), error.message, error.type);
>> }
>>
>> +ovs_mutex_unlock(_map_mutex);
>> return ret;
>> }
>>
>> --
>> 2.14.5
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Celibr%40mellanox.com%7C9af2f119de544768711408d77c779a66%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637114725449215533sdata=VyrLUh6%2BP0ne9dXUSDjME31NBj0tHTV%2FEciNZY%2Baugo%3Dreserved=0
>
>
> --
> hepeng
> ICT
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 08/19] netdev-offload-dpdk: Protect UFID map by mutex
Hi,
cmap support multiple readers and one writer.
since all write operations are performed in the offload thread,
why need a mutex here? Maybe I miss some patches.
Eli Britstein 于2019年12月8日周日 下午9:23写道：
>
> Flow deletion and dumping for statistics collection are called from
> different threads. As a pre-step towards collecting HW statistics,
> protect the UFID map by mutex to make it thread safe.
>
> Signed-off-by: Eli Britstein
> ---
> lib/netdev-offload-dpdk.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index b2ec05cec..5568400b6 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -51,6 +51,7 @@ static struct vlog_rate_limit error_rl =
> VLOG_RATE_LIMIT_INIT(100, 5);
> * A mapping from ufid to dpdk rte_flow.
> */
> static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> +static struct ovs_mutex ufid_map_mutex = OVS_MUTEX_INITIALIZER;
>
> struct ufid_to_rte_flow_data {
> struct cmap_node node;
> @@ -630,8 +631,11 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
> struct rte_flow *rte_flow)
> {
> struct rte_flow_error error;
> -int ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
> +int ret;
> +
> +ovs_mutex_lock(_map_mutex);
>
> +ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
> if (ret == 0) {
> ufid_to_rte_flow_disassociate(ufid);
> VLOG_DBG("%s: removed rte flow %p associated with ufid " UUID_FMT
> "\n",
> @@ -642,6 +646,7 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
> netdev_get_name(netdev), error.message, error.type);
> }
>
> +ovs_mutex_unlock(_map_mutex);
> return ret;
> }
>
> --
> 2.14.5
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
hepeng
ICT
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 3/3] checkpatch: Check spelling in commit messages.
This seems useful as I'm usually making a lot of typing mistakes.
Also, few commonly used words added to the extended dictionary.
Signed-off-by: Ilya Maximets
---
utilities/checkpatch.py | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 1b14ca5d5..3378e8c3e 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -32,7 +32,7 @@ print_file_name = None
checking_file = False
total_line = 0
colors = False
-spellcheck_comments = False
+spellcheck = False
quiet = False
spell_check_dict = None
@@ -84,7 +84,12 @@ def open_spell_check_dict():
'cacheline', 'xlate', 'skiplist', 'idl',
'comparator', 'natting', 'alg', 'pasv', 'epasv',
'wildcard', 'nated', 'amd64', 'x86_64',
- 'recirculation']
+ 'recirculation', 'linux', 'afxdp', 'promisc', 'goto',
+ 'misconfigured', 'misconfiguration', 'checkpatch',
+ 'debian', 'travis', 'cirrus', 'appveyor', 'faq',
+ 'erspan', 'const', 'hotplug', 'addresssanitizer',
+ 'ovsdb', 'dpif', 'veth', 'rhel', 'jsonrpc', 'json',
+ 'syscall', 'lacp', 'ipf', 'skb', 'valgrind']
global spell_check_dict
spell_check_dict = enchant.Dict("en_US")
@@ -372,12 +377,14 @@ def filter_comments(current_line, keep=False):
return sanitized_line
-def check_comment_spelling(line):
-if not spell_check_dict or not spellcheck_comments:
+def check_spelling(line, comment):
+if not spell_check_dict or not spellcheck:
return False
-comment_words = filter_comments(line, True).replace(':', ' ').split(' ')
-for word in comment_words:
+words = filter_comments(line, True) if comment else line
+words = words.replace(':', ' ').split(' ')
+
+for word in words:
skip = False
strword = re.subn(r'\W+', '', word)[0].replace(',', '')
if (len(strword)
@@ -561,7 +568,7 @@ checks = [
{'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None,
'prereq': lambda x: has_comment(x),
- 'check': lambda x: check_comment_spelling(x)},
+ 'check': lambda x: check_spelling(x, True)},
{'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None,
'check': lambda x: empty_return_with_brace(x),
@@ -811,6 +818,9 @@ def ovs_checkpatch_parse(text, filename, author=None,
committer=None):
print_error(
"Remove Gerrit Change-Id's before submitting upstream.")
print("%d: %s\n" % (lineno, line))
+elif spellcheck:
+check_spelling(line, False)
+
elif parse == PARSE_STATE_CHANGE_BODY:
newfile = hunks.match(line)
if newfile:
@@ -873,7 +883,8 @@ Check options:
-l|--skip-leading-whitespace Skips the leading whitespace test
-q|--quiet Only print error and warning information
-s|--skip-signoff-linesTolerate missing Signed-off-by line
--S|--spellcheck-comments Check C comments for possible spelling mistakes
+-S|--spellcheckCheck C comments and commit-message for possible
+ spelling mistakes
-t|--skip-trailing-whitespace Skips the trailing whitespace test"""
% sys.argv[0])
@@ -931,7 +942,7 @@ if __name__ == '__main__':
"skip-leading-whitespace",
"skip-signoff-lines",
"skip-trailing-whitespace",
- "spellcheck-comments",
+ "spellcheck",
"quiet"])
except:
print("Unknown option encountered. Please rerun with -h for help.")
@@ -951,12 +962,12 @@ if __name__ == '__main__':
skip_trailing_whitespace_check = True
elif o in ("-f", "--check-file"):
checking_file = True
-elif o in ("-S", "--spellcheck-comments"):
+elif o in ("-S", "--spellcheck"):
if not open_spell_check_dict():
print("WARNING: The enchant library isn't available.")
print(" Please install python enchant.")
else:
-spellcheck_comments = True
+spellcheck = True
elif o in ("-q", "--quiet"):
quiet = True
else:
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/3] checkpatch: Skip words containing numbers.
Words like 'br0' are common and usually references some code or
database objects that should not be targets for spell checking.
So, it's better to skip all the words that has digits inside instead
of ones that starts with numbers.
Signed-off-by: Ilya Maximets
---
utilities/checkpatch.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 6d95b0e2f..1b14ca5d5 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -396,8 +396,8 @@ def check_comment_spelling(line):
strword[1:].islower()):
skip = True
-# skip words that start with numbers
-if strword.startswith(tuple('0123456789')):
+# skip words containing numbers
+if any(check_char.isdigit() for check_char in strword):
skip = True
if not skip:
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/3] checkpatch: Allow common abbreviations for spell checking.
Abbreviations of Latin expressions like 'i.e.' or 'e.g.' are common
and known by the dictionary. However, our spell checker is not able
to recognize them because it strips dots out of them. To avoid this
issue we could pass non-stripped version of the word to the dictionary
checker too.
Signed-off-by: Ilya Maximets
---
utilities/checkpatch.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 9b79d268f..6d95b0e2f 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -380,7 +380,9 @@ def check_comment_spelling(line):
for word in comment_words:
skip = False
strword = re.subn(r'\W+', '', word)[0].replace(',', '')
-if len(strword) and not spell_check_dict.check(strword.lower()):
+if (len(strword)
+and not spell_check_dict.check(strword.lower())
+and not spell_check_dict.check(word.lower())):
if any([check_char in word
for check_char in ['=', '(', '-', '_', '/', '\'']]):
skip = True
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/3] checkpatch: Spell checker enhancements.
Ilya Maximets (3): checkpatch: Allow common abbreviations for spell checking. checkpatch: Skip words containing numbers. checkpatch: Check spelling in commit messages. utilities/checkpatch.py | 41 +++-- 1 file changed, 27 insertions(+), 14 deletions(-) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/2] Get rid of broken dpif pointer in dp_netdev structure.
Ilya Maximets (2): dpif: Turn dpif_flow_hash function into generic odp_flow_key_hash. dpif-netdev: Get rid of broken dpif pointer in dp_netdev structure. lib/dpif-netdev.c | 12 +--- lib/dpif-netlink.c | 27 --- lib/dpif.c | 16 lib/dpif.h | 4 +--- lib/odp-util.c | 17 + lib/odp-util.h | 2 +- ofproto/ofproto-dpif.c | 2 +- 7 files changed, 33 insertions(+), 47 deletions(-) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] dpif-netdev: Get rid of broken dpif pointer in dp_netdev structure.
This pointer was introduced in July 2014 by commit
6b31e07347ad ("dpif-netdev: Polling threads directly call ofproto upcall
functions.")
and it was broken right from this point because dpif_netdev_open()
updates it on each call with the pointer to a newly allocated
'dpif' structure that becomes invalid on the next dpif_netdev_close().
Since dpif_open/close() always happens asynchronously from different
threads and pointer is not protected by rcu or mutex (it's not even
atomic) it's not possible to safely use it. Thankfully the actual
usage was in repository for less than 3 weeks and was removed by
commit 623540e4617e ("dpif-netdev: Streamline miss handling."). Until
recently this pointer was used in order to pass it to dpif_flow_hash().
Another luck is that dpif_flow_hash() didn't use the 'dpif' argument.
However, we tried to use it while netdev offloading by commit
30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup while
offloading.")
and that unveiled the issue.
Now that all the code that used this pointer was cleaned up we can
just remove it from the structure to avoid possible misuse in the
future.
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ffb3cd420..38d1f12ef 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -303,7 +303,6 @@ struct pmd_auto_lb {
struct dp_netdev {
const struct dpif_class *const class;
const char *const name;
-struct dpif *dpif;
struct ovs_refcount ref_cnt;
atomic_flag destroyed;
@@ -1618,7 +1617,6 @@ dpif_netdev_open(const struct dpif_class *class, const
char *name,
}
if (!error) {
*dpifp = create_dpif_netdev(dp);
-dp->dpif = *dpifp;
}
ovs_mutex_unlock(_netdev_mutex);
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/2] dpif: Turn dpif_flow_hash function into generic odp_flow_key_hash.
Current implementation of dpif_flow_hash() doesn't depend on datapath
interface and only complicates the callers by forcing them to figure
out what is their current 'dpif'. If we'll need different hashing
for different 'dpif's we'll implement an API for dpif-providers
and each dpif implementation will be able to use their local function
directly without calling it via dpif API.
This change will allow us to not store 'dpif' pointer in the userspace
datapath implementation which is broken and will be removed in next
commits.
This patch moves dpif_flow_hash() to odp-util module and replaces
unused odp_flow_key_hash() by it, along with removing of unused 'dpif'
argument.
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 10 +-
lib/dpif-netlink.c | 27 ---
lib/dpif.c | 16
lib/dpif.h | 4 +---
lib/odp-util.c | 17 +
lib/odp-util.h | 2 +-
ofproto/ofproto-dpif.c | 2 +-
7 files changed, 33 insertions(+), 45 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7ebf4ef87..ffb3cd420 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2984,7 +2984,7 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread
*pmd,
/* If a UFID is not provided, determine one based on the key. */
if (!ufidp && key && key_len
&& !dpif_netdev_flow_from_nlattrs(key, key_len, , false)) {
-dpif_flow_hash(pmd->dp->dpif, , sizeof flow, );
+odp_flow_key_hash(, sizeof flow, );
ufidp =
}
@@ -3203,7 +3203,7 @@ dp_netdev_get_mega_ufid(const struct match *match,
ovs_u128 *mega_ufid)
((uint8_t *)_flow)[i] = ((uint8_t *)>flow)[i] &
((uint8_t *)>wc)[i];
}
-dpif_flow_hash(NULL, _flow, sizeof(struct flow), mega_ufid);
+odp_flow_key_hash(_flow, sizeof masked_flow, mega_ufid);
}
static struct dp_netdev_flow *
@@ -3407,7 +3407,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct
dpif_flow_put *put)
if (put->ufid) {
ufid = *put->ufid;
} else {
-dpif_flow_hash(dpif, , sizeof match.flow, );
+odp_flow_key_hash(, sizeof match.flow, );
}
/* The Netlink encoding of datapath flow keys cannot express
@@ -6615,7 +6615,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd,
ofpbuf_clear(actions);
ofpbuf_clear(put_actions);
-dpif_flow_hash(pmd->dp->dpif, , sizeof match.flow, );
+odp_flow_key_hash(, sizeof match.flow, );
error = dp_netdev_upcall(pmd, packet, , ,
, DPIF_UC_MISS, NULL, actions,
put_actions);
@@ -7152,7 +7152,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
*packets_,
struct dp_packet *packet;
DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
flow_extract(packet, );
-dpif_flow_hash(dp->dpif, , sizeof flow, );
+odp_flow_key_hash(, sizeof flow, );
dp_execute_userspace_action(pmd, packet, should_steal, ,
, , userdata);
}
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e9a6887f7..23e513eff 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -153,7 +153,7 @@ static int dpif_netlink_flow_transact(struct
dpif_netlink_flow *request,
struct ofpbuf **bufp);
static void dpif_netlink_flow_get_stats(const struct dpif_netlink_flow *,
struct dpif_flow_stats *);
-static void dpif_netlink_flow_to_dpif_flow(struct dpif *, struct dpif_flow *,
+static void dpif_netlink_flow_to_dpif_flow(struct dpif_flow *,
const struct dpif_netlink_flow *);
/* One of the dpif channels between the kernel and userspace. */
@@ -1527,7 +1527,7 @@ dpif_netlink_flow_dump_thread_destroy(struct
dpif_flow_dump_thread *thread_)
}
static void
-dpif_netlink_flow_to_dpif_flow(struct dpif *dpif, struct dpif_flow *dpif_flow,
+dpif_netlink_flow_to_dpif_flow(struct dpif_flow *dpif_flow,
const struct dpif_netlink_flow *datapath_flow)
{
dpif_flow->key = datapath_flow->key;
@@ -1542,8 +1542,8 @@ dpif_netlink_flow_to_dpif_flow(struct dpif *dpif, struct
dpif_flow *dpif_flow,
dpif_flow->ufid = datapath_flow->ufid;
} else {
ovs_assert(datapath_flow->key && datapath_flow->key_len);
-dpif_flow_hash(dpif, datapath_flow->key, datapath_flow->key_len,
- _flow->ufid);
+odp_flow_key_hash(datapath_flow->key, datapath_flow->key_len,
+ _flow->ufid);
}
dpif_netlink_flow_get_stats(datapath_flow, _flow->stats);
dpif_flow->attrs.offloaded = false;
@@ -1716,8 +1716,7 @@ dpif_netlink_flow_dump_next(struct dpif_flow_dump_thread
*thread_,
if (dump->up.terse ||
Re: [ovs-dev] [PATCH v1] dpif-netdev: Retrieve dpif_class from struct dp_netdev
On 08.12.2019 15:42, Ilya Maximets wrote:
> On 08.12.2019 15:29, Ophir Munk wrote:
>> In case a pmd pointer (struct dp_netdev_pmd_thread *) needs to retrieve
>> the dpif_class it points at - it can access it as: pmd->dp->class. A
>> second option is to access it as: pmd->dp->dpif->dpif_class. The first
>> option is safe since there is one dp netdev with a constant pointer to
>> the dpif class. The second option is not safe since the pointer
>> pmd->dp->dpif may be changed under the hood, for example, in case there
>> is a call to dpif_open(). One such scenario is when a netdev bridge is
>> running while dumping flows statistics with dpctl in parallel:
>> ovs-appctl dpctl/dump-flows. This commit makes usage of the first
>> safe option instead of the second option.
>>
>> Fixes: 30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup
>> while offloading")
>> Signed-off-by: Ophir Munk
>
>
> Thanks Ophir!
>
> I reproduced the issue locally and this patch will fix current case.
> I'll re-check it and apply to master.
Applied.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v3 0/4] netdev-offload: Prerequisites of vport offloading via DPDK.
Hi Ilya,
I think the root cause for the issues mentioned below is the usage of
pmd->dp->dpif->dpif_class
rather than pmd->dp->class. Please see a fixing patch
https://patchwork.ozlabs.org/patch/1205684/
("dpif-netdev: Retrieve dpif_class from struct dp_netdev ").
When running dpctl statistics there is a call to dpif_open(). As part of this
call dp->dpif is assigned a new dpif pointer value.
However pmd->dp->dpif should assume that dp->dpif is fixed throughout its life
time.
Hence it is better to access the dpif_class through fixed pointers that do not
change (pmd->dp->class).
With the fixing patch Eli issues are fixes.
Also when I updated your RFCs series to use pmd->dp->class - there is no more
crash and everything works fine.
Ilya - can you please update the RFC series and send them as V1 patches?
I will test them again and hopefully will be able to ACK them.
Regards,
Ophir
> -Original Message-
> From: Eli Britstein
> Sent: Sunday, December 8, 2019 2:29 PM
> To: Ilya Maximets ; Ophir Munk
> ; ovs-dev@openvswitch.org
> Cc: Roni Bar Yanai ; Simon Horman
> ; Ameer Mahagneh
>
> Subject: Re: [RFC v3 0/4] netdev-offload: Prerequisites of vport offloading
> via
> DPDK.
>
>
> On 12/6/2019 4:34 PM, Ilya Maximets wrote:
> > On 06.12.2019 14:09, Ophir Munk wrote:
> >> Hi Ilya,
> >> I applied this series on master ("dpdk: Update to use DPDK 19.11.") and
> PINGed between two "dpdk" ports (hw-offload=true).
> >> It worked fine.
> >> Then I watched flow statistics (dpif based) by executing the command in
> (1):
> >> (1) watch ovs-appctl dpif/dump-flows -m It worked fine.
> >> Then I watched flow statistics (dpctl based) by executing the command in
> (2):
> >> (2) watch ovs-appctl dpctl/dump-flows In this case OVS crashed after
> >> a few seconds.
> >>
> >> When inspecting the calls trace I notice that pmd->dp->dpif->dpif_class-
> >type is a corrupted memory address.
> >>
> >> (gdb) bt
> >> #0 0x00c26fda in dpif_normalize_type (type=0x7372 >> 0x7372 out of bounds>) at lib/dpif.c:517
> >> #1 0x00c19864 in dp_netdev_flow_offload_put
> >> (offload=offload@entry=0x7f11fc00b790) at lib/dpif-netdev.c:2378
> >> #2 0x00c19ca8 in dp_netdev_flow_offload_main
> >> (data=) at lib/dpif-netdev.c:2467
> >> #3 0x00ca3c9d in ovsthread_wrapper (aux_=)
> at
> >> lib/ovs-thread.c:383
> >> #4 0x7f12a2a08e25 in start_thread () from /lib64/libpthread.so.0
> >> #5 0x7f12a222c34d in clone () from /lib64/libc.so.6
> >>
> >> This scenario repeats whenever running the command in (2) and when
> the code calls dpif_normalize_type().
> >> It seems to me that the memory corruption was there for a while but only
> now it is exposed due to your rfc series.
> >>
> >> In order to observe this memory corruption I added the following
> printouts.
> >> I tested it with hw-offload=false:
> >> ovs-vsctl set Open_vSwitch . other_config:hw-offload=false I tested
> >> it on latest master without your rfc series.
> >>
> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> >> 1e54936..18a804a 100644
> >> --- a/lib/dpif-netdev.c
> >> +++ b/lib/dpif-netdev.c
> >> @@ -3625,6 +3625,10 @@ dpif_netdev_flow_dump_next(struct
> dpif_flow_dump_thread *thread_,
> >> }
> >> }
> >>
> >> +VLOG_ERR("pmd->dp=%p pmd->dp->dpif=%p pmd->dp->dpif-
> >dpif_class=%p \
> >> + pmd->dp->dpif->full_name=%s",
> >> + pmd->dp, pmd->dp->dpif, pmd->dp->dpif->dpif_class,
> >> + pmd->dp->dpif->full_name);
> >> do {
> >> for (n_flows = 0; n_flows < flow_limit; n_flows++) {
> >> struct cmap_node *node;
> >>
> >>
> >> Looking at the printouts the memory corruption is observed.
> >>
> >> Initially all printouts are identical and correct as expected.
> >>
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
> >> pmd->dp->dpif->full_name=netdev@ovs-netdev
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
> >> pmd->dp->dpif->full_name=netdev@ovs-netdev
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
> >> pmd->dp->dpif->full_name=netdev@ovs-netdev
> >>
> >> Around this point I executed the dpctl command in (2) and you can notice
> that the pointers pmd->dp->dp_dpif and beyond became modified.
> >>
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x251c790
> >> pmd->dp->dpif->full_name=(null)
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x33c03608
> >> pmd->dp->dpif->full_name=
> >> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
> >> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x2605700
> >> pmd->dp->dpif->full_name=
> >>
Re: [ovs-dev] [PATCH v1] dpif-netdev: Retrieve dpif_class from struct dp_netdev
On 08.12.2019 15:29, Ophir Munk wrote:
> In case a pmd pointer (struct dp_netdev_pmd_thread *) needs to retrieve
> the dpif_class it points at - it can access it as: pmd->dp->class. A
> second option is to access it as: pmd->dp->dpif->dpif_class. The first
> option is safe since there is one dp netdev with a constant pointer to
> the dpif class. The second option is not safe since the pointer
> pmd->dp->dpif may be changed under the hood, for example, in case there
> is a call to dpif_open(). One such scenario is when a netdev bridge is
> running while dumping flows statistics with dpctl in parallel:
> ovs-appctl dpctl/dump-flows. This commit makes usage of the first
> safe option instead of the second option.
>
> Fixes: 30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup
> while offloading")
> Signed-off-by: Ophir Munk
Thanks Ophir!
I reproduced the issue locally and this patch will fix current case.
I'll re-check it and apply to master.
However, the root cause is that dpif_netdev_open() updates the 'dpif'
pointer on each call that ends up in using of a freed 'dpif'. I'm
working on getting rid of this pointer from the dp_netdev strucutre,
so we'll not have same issue again. Also, 'prerequisites' patch-set
still uses this pointer, so I'll need to update it.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1] dpif-netdev: Retrieve dpif_class from struct dp_netdev
In case a pmd pointer (struct dp_netdev_pmd_thread *) needs to retrieve
the dpif_class it points at - it can access it as: pmd->dp->class. A
second option is to access it as: pmd->dp->dpif->dpif_class. The first
option is safe since there is one dp netdev with a constant pointer to
the dpif class. The second option is not safe since the pointer
pmd->dp->dpif may be changed under the hood, for example, in case there
is a call to dpif_open(). One such scenario is when a netdev bridge is
running while dumping flows statistics with dpctl in parallel:
ovs-appctl dpctl/dump-flows. This commit makes usage of the first
safe option instead of the second option.
Fixes: 30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup while
offloading")
Signed-off-by: Ophir Munk
---
lib/dpif-netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 1e54936..7ebf4ef 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2269,7 +2269,7 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread
*pmd,
struct netdev *port;
odp_port_t in_port = flow->flow.in_port.odp_port;
-port = netdev_ports_get(in_port, pmd->dp->dpif->dpif_class);
+port = netdev_ports_get(in_port, pmd->dp->class);
if (port) {
ret = netdev_flow_del(port, >mega_ufid, NULL);
netdev_close(port);
@@ -2410,7 +2410,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item
*offload)
}
info.flow_mark = mark;
-port = netdev_ports_get(in_port, pmd->dp->dpif->dpif_class);
+port = netdev_ports_get(in_port, pmd->dp->class);
if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
netdev_close(port);
goto err_free;
--
2.8.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 10/19] dpif-netdev: Read hw stats during flow_dump_next() call
Bleep bloop. Greetings Eli Britstein, 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 81 characters long (recommended limit is 79) #88 FILE: lib/dpif-netdev.c:3631: non_atomic_ullong_add(_flow->stats.packet_count, stats.n_packets); Lines checked: 112, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 06/19] netdev-offload-dpdk: Framework for actions offload
Bleep bloop. Greetings Eli Britstein, 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 80 characters long (recommended limit is 79) #159 FILE: lib/netdev-offload-dpdk.c:540: flow = netdev_offload_dpdk_mark_rss(, netdev, info->flow_mark); Lines checked: 190, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 04/19] netdev-offload-dpdk: Fix typo
Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-offload-dpdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index e48ac3b63..041b72d53 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -656,7 +656,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
actions.actions, );
if (!flow) {
-VLOG_ERR("%s: rte flow creat error: %u : message : %s\n",
+VLOG_ERR("%s: rte flow create error: %u : message : %s\n",
netdev_get_name(netdev), error.type, error.message);
ret = -1;
goto out;
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 16/19] netdev-offload-dpdk: Support offload of set MAC actions
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 3 +-
lib/netdev-dpdk.c| 15 ++
lib/netdev-offload-dpdk.c| 107 +++
4 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 6cedd7f63..d228493e9 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -392,6 +392,7 @@ Supported actions for hardware offload are:
- Output (a single output, as the last action).
- Drop.
+- Modification of Ethernet (mod_dl_src/mod_dl_dst).
Further Reading
---
diff --git a/NEWS b/NEWS
index d019e066f..3ade86d49 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,8 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
- * Add hardware offload support for output and drop actions (experimental).
+ * Add hardware offload support for output, drop and set MAC actions
+ (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 872a45e75..e67a3dd76 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
}
} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
ds_put_cstr(s, "rte flow drop action\n");
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ||
+ actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) {
+const struct rte_flow_action_set_mac *set_mac = actions->conf;
+
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST
+ ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr);
+if (set_mac) {
+ds_put_format(s,
+ " Set-mac-%s: "ETH_ADDR_FMT"\n",
+ dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr));
+} else {
+ds_put_format(s, " Set-mac-%s = null\n", dirstr);
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index bffb69cad..07f5d4687 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev,
return ret;
}
+struct set_action_info {
+const uint8_t *value, *mask;
+const uint8_t size;
+uint8_t *spec;
+const int attr;
+};
+
+static int
+add_set_flow_action(struct flow_actions *actions,
+struct set_action_info *sa_info_arr,
+size_t sa_info_arr_size)
+{
+int field, i;
+
+for (field = 0; field < sa_info_arr_size; field++) {
+if (sa_info_arr[field].mask) {
+/* DPDK does not support partially masked set actions. In such
+ * case, fail the offload.
+ */
+if (sa_info_arr[field].mask[0] != 0x00 &&
+sa_info_arr[field].mask[0] != 0xFF) {
+VLOG_DBG_RL(_rl,
+"Partial mask is not supported");
+return -1;
+}
+
+for (i = 1; i < sa_info_arr[field].size; i++) {
+if (sa_info_arr[field].mask[i] !=
+sa_info_arr[field].mask[i - 1]) {
+VLOG_DBG_RL(_rl,
+"Partial mask is not supported");
+return -1;
+}
+}
+
+if (sa_info_arr[field].mask[0] == 0x00) {
+/* mask bytes are all 0 - no rewrite action required */
+continue;
+}
+}
+
+memcpy(sa_info_arr[field].spec, sa_info_arr[field].value,
+ sa_info_arr[field].size);
+add_flow_action(actions, sa_info_arr[field].attr,
+sa_info_arr[field].spec);
+}
+
+return 0;
+}
+
+/* Mask is at the midpoint of the data. */
+#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1)
+
+#define SA_INFO(_field, _spec, _attr) { \
+.value = (uint8_t *)>_field, \
+.mask = (masked) ? (uint8_t *)>_field : NULL, \
+.size = sizeof key->_field, \
+.spec = (uint8_t *)&_spec, \
+.attr = _attr }
+
+static int
+parse_set_actions(struct flow_actions *actions,
+ const struct nlattr *set_actions,
+ const size_t set_actions_len,
+ bool masked)
+{
+const struct nlattr *sa;
+unsigned int sleft;
+
+NL_ATTR_FOR_EACH_UNSAFE (sa, sleft, set_actions, set_actions_len) {
+if (nl_attr_type(sa) == OVS_KEY_ATTR_ETHERNET) {
+const struct ovs_key_ethernet *key = nl_attr_get(sa);
+const struct ovs_key_ethernet *mask = masked ?
+
[ovs-dev] [PATCH V3 15/19] netdev-offload-dpdk: Support offload of drop action
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 2 +-
lib/netdev-dpdk.c| 2 ++
lib/netdev-offload-dpdk.c| 4 +---
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index f62ce82af..6cedd7f63 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -391,6 +391,7 @@ Supported protocols for hardware offload matches are:
Supported actions for hardware offload are:
- Output (a single output, as the last action).
+- Drop.
Further Reading
---
diff --git a/NEWS b/NEWS
index c430999a0..d019e066f 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,7 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
- * Add hardware offload support for output actions (experimental).
+ * Add hardware offload support for output and drop actions (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d9a2c2004..872a45e75 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4721,6 +4721,8 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " Port-id = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
+ds_put_cstr(s, "rte flow drop action\n");
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 0b9087192..bffb69cad 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -536,9 +536,7 @@ parse_flow_actions(struct netdev *netdev,
}
if (nl_actions_len == 0) {
-VLOG_DBG_RL(_rl,
-"Unsupported action type drop");
-return -1;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);
}
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 10/19] dpif-netdev: Read hw stats during flow_dump_next() call
From: Ophir Munk
Use netdev flow get API in order to update the statistics of fully
offloaded flows.
Co-authored-by: Eli Britstein
Signed-off-by: Ophir Munk
Reviewed-by: Oz Shlomo
Signed-off-by: Eli Britstein
---
lib/dpif-netdev.c | 47 +--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 1e5493615..37e7e5c38 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -527,6 +527,7 @@ struct dp_netdev_flow {
bool dead;
uint32_t mark; /* Unique flow mark assigned to a flow */
+bool actions_offloaded; /* true if flow is fully offloaded. */
/* Statistics. */
struct dp_netdev_flow_stats stats;
@@ -2409,6 +2410,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item
*offload)
}
}
info.flow_mark = mark;
+info.actions_offloaded = >actions_offloaded;
port = netdev_ports_get(in_port, pmd->dp->dpif->dpif_class);
if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
@@ -3071,8 +3073,8 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow
*netdev_flow,
flow->pmd_id = netdev_flow->pmd_id;
get_dpif_flow_stats(netdev_flow, >stats);
-flow->attrs.offloaded = false;
-flow->attrs.dp_layer = "ovs";
+flow->attrs.offloaded = netdev_flow->actions_offloaded;
+flow->attrs.dp_layer = flow->attrs.offloaded ? "in_hw" : "ovs";
}
static int
@@ -3242,6 +3244,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
flow->dead = false;
flow->batch = NULL;
flow->mark = INVALID_FLOW_MARK;
+flow->actions_offloaded = false;
*CONST_CAST(unsigned *, >pmd_id) = pmd->core_id;
*CONST_CAST(struct flow *, >flow) = match->flow;
*CONST_CAST(ovs_u128 *, >ufid) = *ufid;
@@ -3596,6 +3599,42 @@ dpif_netdev_flow_dump_thread_destroy(struct
dpif_flow_dump_thread *thread_)
free(thread);
}
+static int
+dpif_netdev_offload_used(struct dp_netdev_flow *netdev_flow,
+ struct dp_netdev_pmd_thread *pmd)
+{
+struct dpif_flow_stats stats;
+struct netdev *netdev;
+struct match match;
+struct nlattr *actions;
+struct dpif_flow_attrs attrs;
+struct ofpbuf wbuffer;
+
+ovs_u128 ufid;
+int ret = 0;
+
+netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port,
+ pmd->dp->class);
+if (!netdev) {
+return -1;
+}
+/* get offloaded stats */
+ufid = netdev_flow->mega_ufid;
+ret = netdev_flow_get(netdev, , , , , ,
+ );
+netdev_close(netdev);
+if (ret) {
+return -1;
+}
+if (stats.n_packets) {
+atomic_store_relaxed(_flow->stats.used, pmd->ctx.now / 1000);
+non_atomic_ullong_add(_flow->stats.packet_count,
stats.n_packets);
+non_atomic_ullong_add(_flow->stats.byte_count, stats.n_bytes);
+}
+
+return 0;
+}
+
static int
dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,
struct dpif_flow *flows, int max_flows)
@@ -3636,6 +3675,10 @@ dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread
*thread_,
netdev_flows[n_flows] = CONTAINER_OF(node,
struct dp_netdev_flow,
node);
+/* Read hardware stats in case of hardware offload */
+if (netdev_flows[n_flows]->actions_offloaded) {
+dpif_netdev_offload_used(netdev_flows[n_flows], pmd);
+}
}
/* When finishing dumping the current pmd thread, moves to
* the next. */
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 18/19] netdev-offload-dpdk: Support offload of clone tnl_push/output actions
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 2 ++
NEWS | 4 ++--
lib/netdev-dpdk.c| 14 +
lib/netdev-offload-dpdk.c| 48
4 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index b9be83002..b9aaf26bf 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -394,6 +394,8 @@ Supported actions for hardware offload are:
- Drop.
- Modification of Ethernet (mod_dl_src/mod_dl_dst).
- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
+- Clone with tnl_push and output (a single clone, as the last action,
+ with a single output, as the last nested clone action).
Further Reading
---
diff --git a/NEWS b/NEWS
index 297ca6fff..25125801b 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
- * Add hardware offload support for output, drop and set actions of
- MAC and IPv4 (experimental).
+ * Add hardware offload support for output, drop, set of MAC, IPv4
+ and tunnel push-output actions (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c4606363d..14f5d6f3b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4762,6 +4762,20 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " Set-ttl = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_RAW_ENCAP) {
+const struct rte_flow_action_raw_encap *raw_encap = actions->conf;
+
+ds_put_cstr(s, "rte flow raw-encap action:\n");
+if (raw_encap) {
+ds_put_format(s,
+ " Raw-encap: size=%ld\n",
+ raw_encap->size);
+ds_put_format(s,
+ " Raw-encap: encap=\n");
+ds_put_hex_dump(s, raw_encap->data, raw_encap->size, 0, false);
+} else {
+ds_put_cstr(s, " Raw-encap = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 3fccac956..b0403a085 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -634,6 +634,45 @@ parse_set_actions(struct flow_actions *actions,
return 0;
}
+static int
+parse_clone_actions(struct netdev *netdev,
+struct flow_actions *actions,
+const struct nlattr *clone_actions,
+const size_t clone_actions_len,
+struct offload_info *info)
+{
+const struct nlattr *ca;
+unsigned int cleft;
+
+NL_ATTR_FOR_EACH_UNSAFE (ca, cleft, clone_actions, clone_actions_len) {
+int clone_type = nl_attr_type(ca);
+
+if (clone_type == OVS_ACTION_ATTR_TUNNEL_PUSH) {
+const struct ovs_action_push_tnl *tnl_push = nl_attr_get(ca);
+struct rte_flow_action_raw_encap *raw_encap =
+xzalloc(sizeof *raw_encap);
+
+raw_encap->data = (uint8_t *)tnl_push->header;
+raw_encap->preserve = NULL;
+raw_encap->size = tnl_push->header_len;
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RAW_ENCAP,
+raw_encap);
+} else if (clone_type == OVS_ACTION_ATTR_OUTPUT) {
+if (!(cleft <= NLA_ALIGN(ca->nla_len)) ||
+add_output_action(netdev, actions, ca, info)) {
+return -1;
+}
+} else {
+VLOG_DBG_RL(_rl,
+"Unsupported clone action. clone_type=%d", clone_type);
+return -1;
+}
+}
+
+return 0;
+}
+
static int
parse_flow_actions(struct netdev *netdev,
struct flow_actions *actions,
@@ -661,6 +700,15 @@ parse_flow_actions(struct netdev *netdev,
masked)) {
return -1;
}
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CLONE) {
+const struct nlattr *clone_actions = nl_attr_get(nla);
+size_t clone_actions_len = nl_attr_get_size(nla);
+
+if (!(left <= NLA_ALIGN(nla->nla_len)) ||
+parse_clone_actions(netdev, actions, clone_actions,
+clone_actions_len, info)) {
+return -1;
+}
} else {
VLOG_DBG_RL(_rl,
"Unsupported action type %d", nl_attr_type(nla));
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 09/19] netdev-offload-dpdk: Implement flow get method
Implement the flow get method for DPDK, to get the statistics of the
provided ufid, towards reading statistics of fully offloaded flows.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-offload-dpdk.c | 40
1 file changed, 40 insertions(+)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 5568400b6..10ab9240a 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -705,9 +705,49 @@ netdev_offload_dpdk_init_flow_api(struct netdev *netdev)
return netdev_dpdk_flow_api_supported(netdev) ? 0 : EOPNOTSUPP;
}
+static int
+netdev_offload_dpdk_flow_get(struct netdev *netdev,
+ struct match *match OVS_UNUSED,
+ struct nlattr **actions OVS_UNUSED,
+ const ovs_u128 *ufid,
+ struct dpif_flow_stats *stats,
+ struct dpif_flow_attrs *attrs OVS_UNUSED,
+ struct ofpbuf *buf OVS_UNUSED)
+{
+struct rte_flow_query_count query = { .reset = 1 };
+struct rte_flow_error error;
+struct rte_flow *rte_flow;
+int ret = 0;
+
+ovs_mutex_lock(_map_mutex);
+rte_flow = ufid_to_rte_flow_find(ufid);
+if (!rte_flow) {
+ret = -1;
+goto out;
+}
+
+memset(stats, 0, sizeof *stats);
+ret = netdev_dpdk_rte_flow_query(netdev, rte_flow, , );
+if (ret) {
+VLOG_DBG_RL(_rl,
+"%s: Failed to query ufid "UUID_FMT" flow: %p\n",
+netdev_get_name(netdev), UUID_ARGS((struct uuid *)ufid),
+rte_flow);
+ret = -1;
+goto out;
+}
+stats->n_packets += (query.hits_set) ? query.hits : 0;
+stats->n_bytes += (query.bytes_set) ? query.bytes : 0;
+
+out:
+ovs_mutex_unlock(_map_mutex);
+return ret;
+}
+
const struct netdev_flow_api netdev_offload_dpdk = {
.type = "dpdk_flow_api",
.flow_put = netdev_offload_dpdk_flow_put,
.flow_del = netdev_offload_dpdk_flow_del,
.init_flow_api = netdev_offload_dpdk_init_flow_api,
+.flow_get = netdev_offload_dpdk_flow_get,
};
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 07/19] netdev-dpdk: Introduce rte flow query function
Introduce a rte flow query function as a pre-step towards reading HW
statistics of fully offloaded flows.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-dpdk.c | 25 +
lib/netdev-dpdk.h | 6 ++
2 files changed, 31 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index da1349b69..e63a496c1 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4737,6 +4737,31 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
return flow;
}
+int
+netdev_dpdk_rte_flow_query(struct netdev *netdev,
+ struct rte_flow *rte_flow,
+ struct rte_flow_query_count *query,
+ struct rte_flow_error *error)
+{
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+struct rte_flow_action_count count = {};
+const struct rte_flow_action actions[] = {
+{
+.type = RTE_FLOW_ACTION_TYPE_COUNT,
+.conf = ,
+},
+{
+.type = RTE_FLOW_ACTION_TYPE_END,
+},
+};
+int ret;
+
+ovs_mutex_lock(>mutex);
+ret = rte_flow_query(dev->port_id, rte_flow, actions, query, error);
+ovs_mutex_unlock(>mutex);
+return ret;
+}
+
#define NETDEV_DPDK_CLASS_COMMON\
.is_pmd = true, \
.alloc = netdev_dpdk_alloc, \
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 60631c4f0..ed7cb235a 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -31,6 +31,7 @@ struct rte_flow_error;
struct rte_flow_attr;
struct rte_flow_item;
struct rte_flow_action;
+struct rte_flow_query_count;
void netdev_dpdk_register(void);
void free_dpdk_buf(struct dp_packet *);
@@ -47,6 +48,11 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
const struct rte_flow_item *items,
const struct rte_flow_action *actions,
struct rte_flow_error *error);
+int
+netdev_dpdk_rte_flow_query(struct netdev *netdev,
+ struct rte_flow *rte_flow,
+ struct rte_flow_query_count *query,
+ struct rte_flow_error *error);
#else
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 08/19] netdev-offload-dpdk: Protect UFID map by mutex
Flow deletion and dumping for statistics collection are called from
different threads. As a pre-step towards collecting HW statistics,
protect the UFID map by mutex to make it thread safe.
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index b2ec05cec..5568400b6 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -51,6 +51,7 @@ static struct vlog_rate_limit error_rl =
VLOG_RATE_LIMIT_INIT(100, 5);
* A mapping from ufid to dpdk rte_flow.
*/
static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
+static struct ovs_mutex ufid_map_mutex = OVS_MUTEX_INITIALIZER;
struct ufid_to_rte_flow_data {
struct cmap_node node;
@@ -630,8 +631,11 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
struct rte_flow *rte_flow)
{
struct rte_flow_error error;
-int ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
+int ret;
+
+ovs_mutex_lock(_map_mutex);
+ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, );
if (ret == 0) {
ufid_to_rte_flow_disassociate(ufid);
VLOG_DBG("%s: removed rte flow %p associated with ufid " UUID_FMT "\n",
@@ -642,6 +646,7 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
netdev_get_name(netdev), error.message, error.type);
}
+ovs_mutex_unlock(_map_mutex);
return ret;
}
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 13/19] netdev-offload: Introduce a function to validate same flow api handle
Signed-off-by: Eli Britstein
---
lib/netdev-offload-provider.h | 1 +
lib/netdev-offload.c | 12
2 files changed, 13 insertions(+)
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 4e1c4251d..5a809c0cd 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -89,6 +89,7 @@ struct netdev_flow_api {
int netdev_register_flow_api_provider(const struct netdev_flow_api *);
int netdev_unregister_flow_api_provider(const char *type);
+bool netdev_flow_api_equals(const struct netdev *, const struct netdev *);
#ifdef __linux__
extern const struct netdev_flow_api netdev_offload_tc;
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index ae01acda6..32eab5910 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -156,6 +156,18 @@ netdev_unregister_flow_api_provider(const char *type)
return error;
}
+bool
+netdev_flow_api_equals(const struct netdev *netdev1,
+ const struct netdev *netdev2)
+{
+const struct netdev_flow_api *netdev_flow_api1 =
+ovsrcu_get(const struct netdev_flow_api *, >flow_api);
+const struct netdev_flow_api *netdev_flow_api2 =
+ovsrcu_get(const struct netdev_flow_api *, >flow_api);
+
+return netdev_flow_api1 == netdev_flow_api2;
+}
+
static int
netdev_assign_flow_api(struct netdev *netdev)
{
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 05/19] netdev-dpdk: Improve HW offload flow debuggability
Add debug prints when creating and destroying rte flows, with all the
flow details (attributes, patterns, actions).
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-dpdk.c | 260 ++
lib/netdev-offload-dpdk.c | 207 +---
2 files changed, 264 insertions(+), 203 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 89c73a29b..da1349b69 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4458,9 +4458,252 @@ netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
ovs_mutex_lock(>mutex);
ret = rte_flow_destroy(dev->port_id, rte_flow, error);
ovs_mutex_unlock(>mutex);
+if (!ret) {
+VLOG_DBG_RL(, "Destroy rte_flow %p: netdev=%s, port=%d\n",
+rte_flow, netdev_get_name(netdev), dev->port_id);
+} else {
+VLOG_ERR_RL(, "Destroy rte_flow %p: netdev=%s, port=%d\n"
+"FAILED. error %u : message : %s",
+rte_flow, netdev_get_name(netdev), dev->port_id,
+error->type, error->message);
+}
return ret;
}
+static void
+ds_put_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
+{
+ds_put_format(s,
+ " Attributes: "
+ "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
+ attr->ingress, attr->egress, attr->priority, attr->group,
+ attr->transfer);
+}
+
+static void
+ds_put_flow_pattern(struct ds *s, const struct rte_flow_item *item)
+{
+if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
+const struct rte_flow_item_eth *eth_spec = item->spec;
+const struct rte_flow_item_eth *eth_mask = item->mask;
+
+ds_put_cstr(s, "rte flow eth pattern:\n");
+if (eth_spec) {
+ds_put_format(s,
+ " Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
+ "type=0x%04" PRIx16"\n",
+ ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
+ ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
+ ntohs(eth_spec->type));
+} else {
+ds_put_cstr(s, " Spec = null\n");
+}
+if (eth_mask) {
+ds_put_format(s,
+ " Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
+ "type=0x%04"PRIx16"\n",
+ ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
+ ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
+ ntohs(eth_mask->type));
+} else {
+ds_put_cstr(s, " Mask = null\n");
+}
+} else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+const struct rte_flow_item_vlan *vlan_spec = item->spec;
+const struct rte_flow_item_vlan *vlan_mask = item->mask;
+
+ds_put_cstr(s, "rte flow vlan pattern:\n");
+if (vlan_spec) {
+ds_put_format(s,
+ " Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
+ ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci));
+} else {
+ds_put_cstr(s, " Spec = null\n");
+}
+
+if (vlan_mask) {
+ds_put_format(s,
+ " Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
+ ntohs(vlan_mask->inner_type), ntohs(vlan_mask->tci));
+} else {
+ds_put_cstr(s, " Mask = null\n");
+}
+} else if (item->type == RTE_FLOW_ITEM_TYPE_IPV4) {
+const struct rte_flow_item_ipv4 *ipv4_spec = item->spec;
+const struct rte_flow_item_ipv4 *ipv4_mask = item->mask;
+
+ds_put_cstr(s, "rte flow ipv4 pattern:\n");
+if (ipv4_spec) {
+ds_put_format(s,
+ " Spec: tos=0x%"PRIx8", ttl=%"PRIx8
+ ", proto=0x%"PRIx8
+ ", src="IP_FMT", dst="IP_FMT"\n",
+ ipv4_spec->hdr.type_of_service,
+ ipv4_spec->hdr.time_to_live,
+ ipv4_spec->hdr.next_proto_id,
+ IP_ARGS(ipv4_spec->hdr.src_addr),
+ IP_ARGS(ipv4_spec->hdr.dst_addr));
+} else {
+ds_put_cstr(s, " Spec = null\n");
+}
+if (ipv4_mask) {
+ds_put_format(s,
+ " Mask: tos=0x%"PRIx8", ttl=%"PRIx8
+ ", proto=0x%"PRIx8
+ ", src="IP_FMT", dst="IP_FMT"\n",
+ ipv4_mask->hdr.type_of_service,
+ ipv4_mask->hdr.time_to_live,
+ ipv4_mask->hdr.next_proto_id,
+ IP_ARGS(ipv4_mask->hdr.src_addr),
+ IP_ARGS(ipv4_mask->hdr.dst_addr));
+} else {
+ds_put_cstr(s, " Mask = null\n");
+
[ovs-dev] [PATCH V3 14/19] netdev-offload-dpdk: Support offload of output action
Support offload of output action, also configuring count action for
allowing query statistics of HW offloaded flows.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 17 ---
NEWS | 1 +
lib/netdev-dpdk.c| 22 +
lib/netdev-offload-dpdk.c| 73 +---
4 files changed, 104 insertions(+), 9 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 766a7950c..f62ce82af 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -370,19 +370,28 @@ The flow hardware offload is disabled by default and can
be enabled by::
$ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
-So far only partial flow offload is implemented. Moreover, it only works
-with PMD drivers have the rte_flow action "MARK + RSS" support.
+Matches and actions are programmed into HW to achieve full offload of
+the flow. If not all actions are supported, fallback to partial flow
+offload (offloading matches only). Moreover, it only works with PMD
+drivers that support the configured rte_flow actions.
+Partial flow offload requires support of "MARK + RSS" actions. Full
+hardware offload requires support of the actions listed below.
The validated NICs are:
- Mellanox (ConnectX-4, ConnectX-4 Lx, ConnectX-5)
- Napatech (NT200B01)
-Supported protocols for hardware offload are:
+Supported protocols for hardware offload matches are:
+
- L2: Ethernet, VLAN
-- L3: IPv4, IPv6
+- L3: IPv4
- L4: TCP, UDP, SCTP, ICMP
+Supported actions for hardware offload are:
+
+- Output (a single output, as the last action).
+
Further Reading
---
diff --git a/NEWS b/NEWS
index 2acde3fe8..c430999a0 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
+ * Add hardware offload support for output actions (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 16baae0c4..d9a2c2004 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4699,6 +4699,28 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " RSS = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+ " Count: shared=%d, id=%d\n",
+ count->shared, count->id);
+} else {
+ds_put_cstr(s, " Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+ " Port-id: original=%d, id=%d\n",
+ port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, " Port-id = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 10ab9240a..0b9087192 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -456,20 +456,83 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns
*patterns,
return flow;
}
+static void
+add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
static int
-parse_flow_actions(struct netdev *netdev OVS_UNUSED,
+add_port_id_action(struct flow_actions *actions,
+ struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+int outdev_id;
+
+outdev_id = netdev_dpdk_get_port_id(outdev);
+if (outdev_id < 0) {
+return -1;
+}
+port_id->id = outdev_id;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+return 0;
+}
+
+static int
+add_output_action(struct netdev *netdev,
+ struct flow_actions *actions,
+ const struct nlattr *nla,
+ struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+int ret = 0;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if
[ovs-dev] [PATCH V3 17/19] netdev-offload-dpdk: Support offload of set IPv4 actions
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 4 ++--
lib/netdev-dpdk.c| 24
lib/netdev-offload-dpdk.c| 26 ++
4 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index d228493e9..b9be83002 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -393,6 +393,7 @@ Supported actions for hardware offload are:
- Output (a single output, as the last action).
- Drop.
- Modification of Ethernet (mod_dl_src/mod_dl_dst).
+- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
Further Reading
---
diff --git a/NEWS b/NEWS
index 3ade86d49..297ca6fff 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
- * Add hardware offload support for output, drop and set MAC actions
- (experimental).
+ * Add hardware offload support for output, drop and set actions of
+ MAC and IPv4 (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e67a3dd76..c4606363d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4738,6 +4738,30 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_format(s, " Set-mac-%s = null\n", dirstr);
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ||
+ actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST) {
+const struct rte_flow_action_set_ipv4 *set_ipv4 = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
+ ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-ipv4-%s action:\n", dirstr);
+if (set_ipv4) {
+ds_put_format(s,
+ " Set-ipv4-%s: "IP_FMT"\n",
+ dirstr, IP_ARGS(set_ipv4->ipv4_addr));
+} else {
+ds_put_format(s, " Set-ipv4-%s = null\n", dirstr);
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TTL) {
+const struct rte_flow_action_set_ttl *set_ttl = actions->conf;
+
+ds_put_cstr(s, "rte flow set-ttl action:\n");
+if (set_ttl) {
+ds_put_format(s,
+ " Set-ttl: %d\n", set_ttl->ttl_value);
+} else {
+ds_put_cstr(s, " Set-ttl = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 07f5d4687..3fccac956 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -594,6 +594,32 @@ parse_set_actions(struct flow_actions *actions,
RTE_FLOW_ACTION_TYPE_SET_MAC_DST),
};
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_IPV4) {
+const struct ovs_key_ipv4 *key = nl_attr_get(sa);
+const struct ovs_key_ipv4 *mask = masked ?
+get_mask(sa, struct ovs_key_ipv4) : NULL;
+struct rte_flow_action_set_ipv4 *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_ipv4 *dst = xzalloc(sizeof *dst);
+struct rte_flow_action_set_ttl *ttl = xzalloc(sizeof *ttl);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(ipv4_src, src->ipv4_addr,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC),
+SA_INFO(ipv4_dst, dst->ipv4_addr,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_DST),
+SA_INFO(ipv4_ttl, ttl->ttl_value,
+RTE_FLOW_ACTION_TYPE_SET_TTL),
+};
+
+if (mask && (mask->ipv4_proto || mask->ipv4_tos ||
+mask->ipv4_frag)) {
+VLOG_DBG_RL(_rl, "Unsupported IPv4 set action");
+return -1;
+}
+
if (add_set_flow_action(actions, sa_info_arr,
ARRAY_SIZE(sa_info_arr))) {
return -1;
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 01/19] netdev-offload-dpdk: Refactor flow patterns
Refactor the flow patterns code to a helper function for better
readability and towards supporting more matches.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-offload-dpdk.c | 208 +-
1 file changed, 112 insertions(+), 96 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 96794dc4d..a008e642f 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -410,54 +410,42 @@ add_flow_rss_action(struct flow_actions *actions,
return rss_data;
}
+struct flow_items {
+struct rte_flow_item_eth eth;
+struct rte_flow_item_vlan vlan;
+struct rte_flow_item_ipv4 ipv4;
+union {
+struct rte_flow_item_tcp tcp;
+struct rte_flow_item_udp udp;
+struct rte_flow_item_sctp sctp;
+struct rte_flow_item_icmp icmp;
+};
+};
+
static int
-netdev_offload_dpdk_add_flow(struct netdev *netdev,
- const struct match *match,
- struct nlattr *nl_actions OVS_UNUSED,
- size_t actions_len OVS_UNUSED,
- const ovs_u128 *ufid,
- struct offload_info *info)
+parse_flow_match(struct flow_patterns *patterns,
+ struct flow_items *spec,
+ struct flow_items *mask,
+ const struct match *match)
{
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
-struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
-struct rte_flow *flow;
-struct rte_flow_error error;
uint8_t proto = 0;
-int ret = 0;
-struct flow_items {
-struct rte_flow_item_eth eth;
-struct rte_flow_item_vlan vlan;
-struct rte_flow_item_ipv4 ipv4;
-union {
-struct rte_flow_item_tcp tcp;
-struct rte_flow_item_udp udp;
-struct rte_flow_item_sctp sctp;
-struct rte_flow_item_icmp icmp;
-};
-} spec, mask;
-
-memset(, 0, sizeof spec);
-memset(, 0, sizeof mask);
+
+memset(spec, 0, sizeof *spec);
+memset(mask, 0, sizeof *mask);
/* Eth */
if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
!eth_addr_is_zero(match->wc.masks.dl_dst)) {
-memcpy(, >flow.dl_dst, sizeof spec.eth.dst);
-memcpy(, >flow.dl_src, sizeof spec.eth.src);
-spec.eth.type = match->flow.dl_type;
+memcpy(>eth.dst, >flow.dl_dst, sizeof spec->eth.dst);
+memcpy(>eth.src, >flow.dl_src, sizeof spec->eth.src);
+spec->eth.type = match->flow.dl_type;
-memcpy(, >wc.masks.dl_dst, sizeof mask.eth.dst);
-memcpy(, >wc.masks.dl_src, sizeof mask.eth.src);
-mask.eth.type = match->wc.masks.dl_type;
+memcpy(>eth.dst, >wc.masks.dl_dst, sizeof mask->eth.dst);
+memcpy(>eth.src, >wc.masks.dl_src, sizeof mask->eth.src);
+mask->eth.type = match->wc.masks.dl_type;
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_ETH,
- , );
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH,
+ >eth, >eth);
} else {
/*
* If user specifies a flow (like UDP flow) without L2 patterns,
@@ -466,41 +454,41 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
* NIC (such as XL710) doesn't support that. Below is a workaround,
* which simply matches any L2 pkts.
*/
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
}
/* VLAN */
if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
-spec.vlan.tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
-mask.vlan.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
+spec->vlan.tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
+mask->vlan.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
/* Match any protocols. */
-mask.vlan.inner_type = 0;
+mask->vlan.inner_type = 0;
-add_flow_pattern(, RTE_FLOW_ITEM_TYPE_VLAN,
- , );
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN,
+ >vlan, >vlan);
}
/* IP v4 */
if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
-spec.ipv4.hdr.type_of_service = match->flow.nw_tos;
-spec.ipv4.hdr.time_to_live= match->flow.nw_ttl;
-spec.ipv4.hdr.next_proto_id = match->flow.nw_proto;
-spec.ipv4.hdr.src_addr= match->flow.nw_src;
-spec.ipv4.hdr.dst_addr= match->flow.nw_dst;
+spec->ipv4.hdr.type_of_service = match->flow.nw_tos;
+spec->ipv4.hdr.time_to_live= match->flow.nw_ttl;
+spec->ipv4.hdr.next_proto_id =
[ovs-dev] [PATCH V3 06/19] netdev-offload-dpdk: Framework for actions offload
Currently HW offload is accelerating only the rule matching sequence.
Introduce a framework for offloading rule actions as a pre-step for
processing the rule actions in HW. In case of a failure, fallback to the
legacy partial offload scheme.
Note: a flow will be fully offloaded only if it can process all its
actions in HW.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-offload-dpdk.c | 112 +++---
lib/netdev-offload.h | 1 +
2 files changed, 97 insertions(+), 16 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index c272da340..b2ec05cec 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -28,6 +28,7 @@
#include "uuid.h"
VLOG_DEFINE_THIS_MODULE(netdev_offload_dpdk);
+static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(100, 5);
/* Thread-safety
* =
@@ -424,6 +425,93 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
}
+static struct rte_flow *
+netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
+ struct netdev *netdev,
+ uint32_t flow_mark)
+{
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+const struct rte_flow_attr flow_attr = {
+.group = 0,
+.priority = 0,
+.ingress = 1,
+.egress = 0
+};
+struct rte_flow_error error;
+struct rte_flow *flow;
+
+add_flow_mark_rss_actions(, flow_mark, netdev);
+
+flow = netdev_dpdk_rte_flow_create(netdev, _attr,
+ patterns->items,
+ actions.actions, );
+
+if (!flow) {
+VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
+ netdev_get_name(netdev), error.message, error.type);
+}
+
+free_flow_actions();
+return flow;
+}
+
+static int
+parse_flow_actions(struct netdev *netdev OVS_UNUSED,
+ struct flow_actions *actions,
+ struct nlattr *nl_actions,
+ size_t nl_actions_len,
+ struct offload_info *info OVS_UNUSED)
+{
+struct nlattr *nla;
+size_t left;
+
+NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
+VLOG_DBG_RL(_rl,
+"Unsupported action type %d", nl_attr_type(nla));
+return -1;
+}
+
+if (nl_actions_len == 0) {
+VLOG_DBG_RL(_rl,
+"Unsupported action type drop");
+return -1;
+}
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+return 0;
+}
+
+static struct rte_flow *
+netdev_offload_dpdk_actions(struct netdev *netdev,
+struct flow_patterns *patterns,
+struct nlattr *nl_actions,
+size_t actions_len,
+struct offload_info *info)
+{
+const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+struct rte_flow *flow = NULL;
+struct rte_flow_error error;
+int ret;
+
+ret = parse_flow_actions(netdev, , nl_actions, actions_len, info);
+if (ret) {
+goto out;
+}
+flow = netdev_dpdk_rte_flow_create(netdev, _attr, patterns->items,
+ actions.actions, );
+if (!flow) {
+VLOG_ERR("%s: Failed to create flow: %s (%u)\n",
+ netdev_get_name(netdev), error.message, error.type);
+}
+if (flow && info->actions_offloaded) {
+*info->actions_offloaded = true;
+}
+out:
+free_flow_actions();
+return flow;
+}
+
static int
netdev_offload_dpdk_add_flow(struct netdev *netdev,
const struct match *match,
@@ -432,16 +520,8 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
const ovs_u128 *ufid,
struct offload_info *info)
{
-const struct rte_flow_attr flow_attr = {
-.group = 0,
-.priority = 0,
-.ingress = 1,
-.egress = 0
-};
struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
-struct flow_actions actions = { .actions = NULL, .cnt = 0 };
struct rte_flow *flow;
-struct rte_flow_error error;
int ret = 0;
ret = parse_flow_match(, match);
@@ -450,15 +530,16 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
goto out;
}
-add_flow_mark_rss_actions(, info->flow_mark, netdev);
-
-flow = netdev_dpdk_rte_flow_create(netdev, _attr,
- patterns.items,
- actions.actions, );
+flow = netdev_offload_dpdk_actions(netdev, , nl_actions,
+ actions_len, info);
+if (!flow) {
+/* if we failed to offload the rule
[ovs-dev] [PATCH V3 03/19] netdev-offload-dpdk: Dynamically allocate pattern items
Instead of statically allocated pattern items on the stack, dynamically
allocate only the required items while parsing the matches, to simplify
the parsing and make it self-contained, without need of external types,
making it easier to support more matches in the future.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-offload-dpdk.c | 206 ++
1 file changed, 118 insertions(+), 88 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index c3b595a0a..e48ac3b63 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -374,6 +374,24 @@ add_flow_action(struct flow_actions *actions, enum
rte_flow_action_type type,
actions->cnt++;
}
+static void
+free_flow_patterns(struct flow_patterns *patterns)
+{
+int i;
+
+for (i = 0; i < patterns->cnt; i++) {
+if (patterns->items[i].spec) {
+free((void *)patterns->items[i].spec);
+}
+if (patterns->items[i].mask) {
+free((void *)patterns->items[i].mask);
+}
+}
+free(patterns->items);
+patterns->items = NULL;
+patterns->cnt = 0;
+}
+
static void
free_flow_actions(struct flow_actions *actions)
{
@@ -389,42 +407,30 @@ free_flow_actions(struct flow_actions *actions)
actions->cnt = 0;
}
-struct flow_items {
-struct rte_flow_item_eth eth;
-struct rte_flow_item_vlan vlan;
-struct rte_flow_item_ipv4 ipv4;
-union {
-struct rte_flow_item_tcp tcp;
-struct rte_flow_item_udp udp;
-struct rte_flow_item_sctp sctp;
-struct rte_flow_item_icmp icmp;
-};
-};
-
static int
parse_flow_match(struct flow_patterns *patterns,
- struct flow_items *spec,
- struct flow_items *mask,
const struct match *match)
{
+uint8_t *next_proto_mask = NULL;
uint8_t proto = 0;
-memset(spec, 0, sizeof *spec);
-memset(mask, 0, sizeof *mask);
-
/* Eth */
if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
!eth_addr_is_zero(match->wc.masks.dl_dst)) {
-memcpy(>eth.dst, >flow.dl_dst, sizeof spec->eth.dst);
-memcpy(>eth.src, >flow.dl_src, sizeof spec->eth.src);
-spec->eth.type = match->flow.dl_type;
+struct rte_flow_item_eth *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
-memcpy(>eth.dst, >wc.masks.dl_dst, sizeof mask->eth.dst);
-memcpy(>eth.src, >wc.masks.dl_src, sizeof mask->eth.src);
-mask->eth.type = match->wc.masks.dl_type;
+memcpy(>dst, >flow.dl_dst, sizeof spec->dst);
+memcpy(>src, >flow.dl_src, sizeof spec->src);
+spec->type = match->flow.dl_type;
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH,
- >eth, >eth);
+memcpy(>dst, >wc.masks.dl_dst, sizeof mask->dst);
+memcpy(>src, >wc.masks.dl_src, sizeof mask->src);
+mask->type = match->wc.masks.dl_type;
+
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
} else {
/*
* If user specifies a flow (like UDP flow) without L2 patterns,
@@ -438,36 +444,45 @@ parse_flow_match(struct flow_patterns *patterns,
/* VLAN */
if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
-spec->vlan.tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
-mask->vlan.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
+struct rte_flow_item_vlan *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+spec->tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
+mask->tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
/* Match any protocols. */
-mask->vlan.inner_type = 0;
+mask->inner_type = 0;
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN,
- >vlan, >vlan);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, spec, mask);
}
/* IP v4 */
if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
-spec->ipv4.hdr.type_of_service = match->flow.nw_tos;
-spec->ipv4.hdr.time_to_live= match->flow.nw_ttl;
-spec->ipv4.hdr.next_proto_id = match->flow.nw_proto;
-spec->ipv4.hdr.src_addr= match->flow.nw_src;
-spec->ipv4.hdr.dst_addr= match->flow.nw_dst;
+struct rte_flow_item_ipv4 *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
-mask->ipv4.hdr.type_of_service = match->wc.masks.nw_tos;
-mask->ipv4.hdr.time_to_live= match->wc.masks.nw_ttl;
-mask->ipv4.hdr.next_proto_id = match->wc.masks.nw_proto;
-mask->ipv4.hdr.src_addr= match->wc.masks.nw_src;
-mask->ipv4.hdr.dst_addr= match->wc.masks.nw_dst;
+spec->hdr.type_of_service = match->flow.nw_tos;
+
[ovs-dev] [PATCH V3 19/19] netdev-offload-dpdk: Support offload of set TCP/UDP ports actions
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
Documentation/howto/dpdk.rst | 1 +
NEWS | 4 ++--
lib/netdev-dpdk.c| 14 ++
lib/netdev-offload-dpdk.c| 34 ++
4 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index b9aaf26bf..df863e4f1 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -396,6 +396,7 @@ Supported actions for hardware offload are:
- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
- Clone with tnl_push and output (a single clone, as the last action,
with a single output, as the last nested clone action).
+- Modification of TCP/UDP (mod_tp_src/mod_tp_dst).
Further Reading
---
diff --git a/NEWS b/NEWS
index 25125801b..d01b8983b 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
* DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
* Add support for DPDK 19.11.
- * Add hardware offload support for output, drop, set of MAC, IPv4
- and tunnel push-output actions (experimental).
+ * Add hardware offload support for output, drop, set of MAC, IPv4,
+ TCP/UDP ports and tunnel push-output actions (experimental).
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 14f5d6f3b..377e8ae07 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4776,6 +4776,20 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " Raw-encap = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC ||
+ actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST) {
+const struct rte_flow_action_set_tp *set_tp = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST
+ ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-tcp/udp-port-%s action:\n", dirstr);
+if (set_tp) {
+ds_put_format(s,
+ " Set-%s-tcp/udp-port: %"PRIu16"\n",
+ dirstr, ntohs(set_tp->port));
+} else {
+ds_put_format(s, " Set-%s-tcp/udp-port = null\n", dirstr);
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
}
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index b0403a085..6a5d00276 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -620,6 +620,40 @@ parse_set_actions(struct flow_actions *actions,
return -1;
}
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_TCP) {
+const struct ovs_key_tcp *key = nl_attr_get(sa);
+const struct ovs_key_tcp *mask = masked ?
+get_mask(sa, struct ovs_key_tcp) : NULL;
+struct rte_flow_action_set_tp *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_tp *dst = xzalloc(sizeof *dst);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(tcp_src, src->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC),
+SA_INFO(tcp_dst, dst->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_DST),
+};
+
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_UDP) {
+const struct ovs_key_udp *key = nl_attr_get(sa);
+const struct ovs_key_udp *mask = masked ?
+get_mask(sa, struct ovs_key_udp) : NULL;
+struct rte_flow_action_set_tp *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_tp *dst = xzalloc(sizeof *dst);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(udp_src, src->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC),
+SA_INFO(udp_dst, dst->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_DST),
+};
+
if (add_set_flow_action(actions, sa_info_arr,
ARRAY_SIZE(sa_info_arr))) {
return -1;
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 11/19] dpif-netdev: Populate dpif class field in offload struct
Populate dpif class field in offload struct to be used in offloading
flow put.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/dpif-netdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 37e7e5c38..1d1de94de 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2373,6 +2373,7 @@ static int
dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
{
struct dp_netdev_pmd_thread *pmd = offload->pmd;
+const struct dpif_class *dpif_class = pmd->dp->class;
struct dp_netdev_flow *flow = offload->flow;
odp_port_t in_port = flow->flow.in_port.odp_port;
bool modification = offload->op == DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
@@ -2411,6 +2412,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item
*offload)
}
info.flow_mark = mark;
info.actions_offloaded = >actions_offloaded;
+info.dpif_class = dpif_class;
port = netdev_ports_get(in_port, pmd->dp->dpif->dpif_class);
if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 00/19] netdev datapath actions offload
Currently, netdev datapath offload only accelerates the flow match sequence by associating a mark per flow. This series introduces the full offload of netdev datapath flows by having the HW also perform the flow actions. This series adds HW offload for output, drop, set MAC, set IPv4, set TCP/UDP ports and tunnel push actions using the DPDK rte_flow API. v1 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/614511472 v2 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/619262148 v3 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/622253870 v1-v2: - fallback to mark/rss in case of any failure of action offload. - dp_layer changed to "in_hw" in case the flow is offloaded. - using netdev_ports_get instead of dp_netdev_lookup_port in stats reading. - using flow_dump_next API instead of a new API. - validity checks for last action of output and clone. - count action should be done for drop as well. - validity check for output action. - added doc in Documentation/howto/dpdk.rst. v2-v3: - removed refactoring for netdev-offload-dpdk-flow.c file. - dropped flush commits. - dbg prints rate-limited, and outside lock. - added validity check for output action - same flow_api handle for both netdevs. - added a mutex to protect possible concurrent deletion/querying of a flow. - statistics are collected using flow_get API. - added more documentation in Documentation/howto/dpdk.rst. Eli Britstein (18): netdev-offload-dpdk: Refactor flow patterns netdev-offload-dpdk: Refactor action items freeing scheme netdev-offload-dpdk: Dynamically allocate pattern items netdev-offload-dpdk: Fix typo netdev-dpdk: Improve HW offload flow debuggability netdev-offload-dpdk: Framework for actions offload netdev-dpdk: Introduce rte flow query function netdev-offload-dpdk: Protect UFID map by mutex netdev-offload-dpdk: Implement flow get method dpif-netdev: Populate dpif class field in offload struct netdev-dpdk: Getter function for dpdk port id API netdev-offload: Introduce a function to validate same flow api handle netdev-offload-dpdk: Support offload of output action netdev-offload-dpdk: Support offload of drop action netdev-offload-dpdk: Support offload of set MAC actions netdev-offload-dpdk: Support offload of set IPv4 actions netdev-offload-dpdk: Support offload of clone tnl_push/output actions netdev-offload-dpdk: Support offload of set TCP/UDP ports actions Ophir Munk (1): dpif-netdev: Read hw stats during flow_dump_next() call Documentation/howto/dpdk.rst | 23 +- NEWS | 2 + lib/dpif-netdev.c | 49 ++- lib/netdev-dpdk.c | 394 + lib/netdev-dpdk.h | 8 + lib/netdev-offload-dpdk.c | 954 +++--- lib/netdev-offload-provider.h | 1 + lib/netdev-offload.c | 12 + lib/netdev-offload.h | 1 + 9 files changed, 1091 insertions(+), 353 deletions(-) -- 2.14.5 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 02/19] netdev-offload-dpdk: Refactor action items freeing scheme
Action item data structures are pointed by rte_flow_action items.
Refactor the code to free the data structures when freeing the
rte_flow_action items, allowing simpler future actions simpler to add to
the code.
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 92 ++-
1 file changed, 52 insertions(+), 40 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index a008e642f..c3b595a0a 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum
rte_flow_action_type type,
actions->cnt++;
}
-struct action_rss_data {
-struct rte_flow_action_rss conf;
-uint16_t queue[0];
-};
-
-static struct action_rss_data *
-add_flow_rss_action(struct flow_actions *actions,
-struct netdev *netdev)
+static void
+free_flow_actions(struct flow_actions *actions)
{
int i;
-struct action_rss_data *rss_data;
-
-rss_data = xmalloc(sizeof *rss_data +
- netdev_n_rxq(netdev) * sizeof rss_data->queue[0]);
-*rss_data = (struct action_rss_data) {
-.conf = (struct rte_flow_action_rss) {
-.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
-.level = 0,
-.types = 0,
-.queue_num = netdev_n_rxq(netdev),
-.queue = rss_data->queue,
-.key_len = 0,
-.key = NULL
-},
-};
-/* Override queue array with default. */
-for (i = 0; i < netdev_n_rxq(netdev); i++) {
- rss_data->queue[i] = i;
+for (i = 0; i < actions->cnt; i++) {
+if (actions->actions[i].conf) {
+free((void *)actions->actions[i].conf);
+}
}
-
-add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, _data->conf);
-
-return rss_data;
+free(actions->actions);
+actions->actions = NULL;
+actions->cnt = 0;
}
struct flow_items {
@@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns,
return 0;
}
+static void
+add_flow_mark_rss_actions(struct flow_actions *actions,
+ uint32_t flow_mark,
+ struct netdev *netdev)
+{
+struct rte_flow_action_mark *mark;
+struct action_rss_data {
+struct rte_flow_action_rss conf;
+uint16_t queue[0];
+} *rss_data;
+int i;
+
+mark = xzalloc(sizeof *mark);
+
+mark->id = flow_mark;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark);
+
+rss_data = xmalloc(sizeof *rss_data +
+ netdev_n_rxq(netdev) * sizeof rss_data->queue[0]);
+*rss_data = (struct action_rss_data) {
+.conf = (struct rte_flow_action_rss) {
+.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
+.level = 0,
+.types = 0,
+.queue_num = netdev_n_rxq(netdev),
+.queue = rss_data->queue,
+.key_len = 0,
+.key = NULL
+},
+};
+
+/* Override queue array with default. */
+for (i = 0; i < netdev_n_rxq(netdev); i++) {
+ rss_data->queue[i] = i;
+}
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, _data->conf);
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+}
+
static int
netdev_offload_dpdk_add_flow(struct netdev *netdev,
const struct match *match,
@@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
goto out;
}
-struct rte_flow_action_mark mark;
-struct action_rss_data *rss;
-
-mark.id = info->flow_mark;
-add_flow_action(, RTE_FLOW_ACTION_TYPE_MARK, );
-
-rss = add_flow_rss_action(, netdev);
-add_flow_action(, RTE_FLOW_ACTION_TYPE_END, NULL);
+add_flow_mark_rss_actions(, info->flow_mark, netdev);
flow = netdev_dpdk_rte_flow_create(netdev, _attr,
patterns.items,
actions.actions, );
-free(rss);
if (!flow) {
VLOG_ERR("%s: rte flow creat error: %u : message : %s\n",
netdev_get_name(netdev), error.type, error.message);
@@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
out:
free(patterns.items);
-free(actions.actions);
+free_flow_actions();
return ret;
}
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 12/19] netdev-dpdk: Getter function for dpdk port id API
Add a getter function for using the dpdk port id outside the scope of
netdev-dpdk.c to be used for HW offload.
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
lib/netdev-dpdk.c | 18 ++
lib/netdev-dpdk.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e63a496c1..16baae0c4 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4426,6 +4426,24 @@ unlock:
return err;
}
+int
+netdev_dpdk_get_port_id(struct netdev *netdev)
+{
+struct netdev_dpdk *dev;
+int ret = -1;
+
+if (!is_dpdk_class(netdev->netdev_class)) {
+goto out;
+}
+
+dev = netdev_dpdk_cast(netdev);
+ovs_mutex_lock(>mutex);
+ret = dev->port_id;
+ovs_mutex_unlock(>mutex);
+out:
+return ret;
+}
+
bool
netdev_dpdk_flow_api_supported(struct netdev *netdev)
{
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index ed7cb235a..65f6eedf3 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -53,6 +53,8 @@ netdev_dpdk_rte_flow_query(struct netdev *netdev,
struct rte_flow *rte_flow,
struct rte_flow_query_count *query,
struct rte_flow_error *error);
+int
+netdev_dpdk_get_port_id(struct netdev *netdev);
#else
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-controller: Run I-P engine even when no SB txn is available.
On Sat, Dec 7, 2019 at 4:53 PM Han Zhou wrote: > > > > On Sat, Dec 7, 2019 at 3:21 AM Dumitru Ceara wrote: > > > > On Sat, Dec 7, 2019 at 1:36 AM Han Zhou wrote: > > > > > > Thanks Dumitru for the patch. Please see my comments below. > > > > > > > Hi Han, > > > > Thanks for the review. > > > > > On Fri, Dec 6, 2019 at 5:38 AM Dumitru Ceara wrote: > > > > > > > > If the last ovn-controller main loop run started a transaction to the > > > > Southbound DB and the transaction is still in progress, ovn-controller > > > > will not call engine_run(). In case there were changes to the DB, > > > > engine_need_run() would return true which would trigger an immediate > > > > forced recompute in the next processing loop iteration. > > > > > > > > However, there are scenarios when updates can be processed incrementally > > > > even if no Southbound DB transaction is available. One example, often > > > > seen in the field, is when the MAC_Binding table is updated. Currently > > > > such an update received while a transaction is still committing would > > > > trigger a forced recompute. > > > > > > > > To minimize the number of forced recomputes, we now call > > > > engine_run(false), i.e., try to process updates incrementally without > > > > allowing recompute, also when ovnsb_idl_txn == NULL. This is safe > > > > because ovnsb_idl_txn is not used by change_handlers and run handlers > > > > are not allowed to execute when calling engine_run(false). > > > > > > > This patch assumes we will never need to do SB updates when handling > > > changes incrementally. There are two problems: > > > 1. I don't see a reason why a change_handler (for doing incremental > > > compute) can't update SB DB. For example Numan mentioned about working on > > > incremental processing for port-bindings on local chassis. I believe > > > those change handling would trigger SB updates, e.g. claiming/releasing a > > > port. > > > 2. Even if we can keep this assumption and never do incremental > > > processing for changes that require updating SB DB, how do we ensure this > > > constraint is enforced in coding? > > > > > > > I think my commit message is not clear enough. I didn't mean that > > change handlers will never perform SB updates. What I meant was that > > if a change handler needs to perform SB updates it needs to validate > > that ovnsb_idl_txn != NULL and if it's NULL return "false" to trigger > > a recompute. At this moment we have no change handler that updates the > > SB database but as you pointed out we might have them in the future. > > Then, the new change handlers will have to make sure that > > ovnsb_idl_txn is not NULL. > > > > OK, maybe I missed the point. Now let me rephrase your idea. > This patch is to do only incremental processing when txn is NULL, which means > executing change handlers instead of run() methods. Assume some change > handlers need to update SB DB, some don't. > If the incoming change (when txn is NULL) doesn't trigger SB update, then > change handlers can complete without aborting. - this seems to happen in most > cases. > If the incoming change happen to require another SB update, since txn is > NULL, the change handlers that are supposed to do the update will need to > return false and result in aborting. - this may not happen very often, so the > approach of this patch should be effective. Exactly. > > > > For 2), it may be easy. We could hide the interface get_context(), and > > > always pass the context to xxx_run() interface, so that ovnsb_idl_txn can > > > be accessed by recompute but never by change_handlers. > > > What I am really concerned is 1). I think the long term solution is to > > > make the OVSDB change tracking available across iterations. Maybe it is > > > not a trivial work. > > > > > > > As mentioned above, I don't think we need this as long as the contract > > of get_context() is clear and the users know to check for NULL txn > > pointers in change handlers. > > > Agree, but I'd suggest to document the patter more clearly, that a run() > handler never expects NULL txn, and a change handler need to check if txn is > NULL before using it. If it is NULL, it should return false (can't process > the change). I'll send a v2 soon and document this requirement better. Thanks, Dumitru > > > > If we believe there is a big performance gain here and we are sure we > > > don't need to update SB DB in change handlers (in short term), we may > > > proceed with this approach as long as 2) is addressed, and revert this > > > whenever the long term solution (OVSDB change tracking over iterations) > > > is ready. > > > > There's no real reason to not try to process changes incrementally > > even when a SB txn is in progress (ovnsb_idl_txn == NULL). We use the > > same reasoning for !ofctrl_can_put() in order to avoid forced > > recomputes. We saw it on customer deployments that MAC_Binding updates > > received while a SB transaction was still in progress were
Re: [ovs-dev] [RFC v3 0/4] netdev-offload: Prerequisites of vport offloading via DPDK.
On 12/6/2019 4:34 PM, Ilya Maximets wrote:
> On 06.12.2019 14:09, Ophir Munk wrote:
>> Hi Ilya,
>> I applied this series on master ("dpdk: Update to use DPDK 19.11.") and
>> PINGed between two "dpdk" ports (hw-offload=true).
>> It worked fine.
>> Then I watched flow statistics (dpif based) by executing the command in (1):
>> (1) watch ovs-appctl dpif/dump-flows -m
>> It worked fine.
>> Then I watched flow statistics (dpctl based) by executing the command in (2):
>> (2) watch ovs-appctl dpctl/dump-flows
>> In this case OVS crashed after a few seconds.
>>
>> When inspecting the calls trace I notice that
>> pmd->dp->dpif->dpif_class->type is a corrupted memory address.
>>
>> (gdb) bt
>> #0 0x00c26fda in dpif_normalize_type (type=0x7372 > out of bounds>) at lib/dpif.c:517
>> #1 0x00c19864 in dp_netdev_flow_offload_put
>> (offload=offload@entry=0x7f11fc00b790) at lib/dpif-netdev.c:2378
>> #2 0x00c19ca8 in dp_netdev_flow_offload_main (data=)
>> at lib/dpif-netdev.c:2467
>> #3 0x00ca3c9d in ovsthread_wrapper (aux_=) at
>> lib/ovs-thread.c:383
>> #4 0x7f12a2a08e25 in start_thread () from /lib64/libpthread.so.0
>> #5 0x7f12a222c34d in clone () from /lib64/libc.so.6
>>
>> This scenario repeats whenever running the command in (2) and when the code
>> calls dpif_normalize_type().
>> It seems to me that the memory corruption was there for a while but only now
>> it is exposed due to your rfc series.
>>
>> In order to observe this memory corruption I added the following printouts.
>> I tested it with hw-offload=false:
>> ovs-vsctl set Open_vSwitch . other_config:hw-offload=false
>> I tested it on latest master without your rfc series.
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 1e54936..18a804a 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3625,6 +3625,10 @@ dpif_netdev_flow_dump_next(struct
>> dpif_flow_dump_thread *thread_,
>> }
>> }
>>
>> +VLOG_ERR("pmd->dp=%p pmd->dp->dpif=%p pmd->dp->dpif->dpif_class=%p \
>> + pmd->dp->dpif->full_name=%s",
>> + pmd->dp, pmd->dp->dpif, pmd->dp->dpif->dpif_class,
>> + pmd->dp->dpif->full_name);
>> do {
>> for (n_flows = 0; n_flows < flow_limit; n_flows++) {
>> struct cmap_node *node;
>>
>>
>> Looking at the printouts the memory corruption is observed.
>>
>> Initially all printouts are identical and correct as expected.
>>
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
>> pmd->dp->dpif->full_name=netdev@ovs-netdev
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
>> pmd->dp->dpif->full_name=netdev@ovs-netdev
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x2525e70 pmd->dp->dpif->dpif_class=0xef3e80
>> pmd->dp->dpif->full_name=netdev@ovs-netdev
>>
>> Around this point I executed the dpctl command in (2) and you can notice
>> that the pointers pmd->dp->dp_dpif and beyond became modified.
>>
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x251c790
>> pmd->dp->dpif->full_name=(null)
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x33c03608
>> pmd->dp->dpif->full_name=
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0x2605700
>> pmd->dp->dpif->full_name=
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x25c3280 pmd->dp->dpif->dpif_class=0xe784250b
>> pmd->dp->dpif->full_name=memseg-2048k-0-3
>> dpif_netdev|ERR|pmd->dp=0x7fdfea3ea010 pmd->dp->dpif=0x2602430
>> pmd->dp->dpif->dpif_class=0xef3e80 pmd->dp->dpif->full_name=netdev@ovs-netdev
>> dpif_netdev(revalidator82)|ERR|pmd->dp=0x7fdfea3ea010
>> pmd->dp->dpif=0x2602430 pmd->dp->dpif->dpif_class=0x2606200
>> pmd->dp->dpif->full_name=(null)
>>
>> The same happens if I test it with hw-offload=true.
>>
>> I hope this scenario can be recreated on other setups as well.
> This is interesting. I'll try to reproduce this issue on my setup.
Hi Ilya,
I see that this bug is not only with this new patch-set, but now also
came into effect in master, with acceptance of
30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup while
offloading.")
- port = dp_netdev_lookup_port(pmd->dp, in_port);
+ port = netdev_ports_get(in_port, pmd->dp->dpif->dpif_class);
As of the corruption, the "port" is not retrieved by netdev_ports_get,
and the offloading is not done. I have reverted this commits in my tests.
Though this commit is correct (but should not have any effect before
vport offloading), maybe we should revert it until this corruption bug
is fixed.
Thanks,
Eli
>
>> I will look more into it.
> Thanks!
>
>
[ovs-dev] [ovs-dev, v2] datapath-windows: Don't delete internal port
According to the microsoft doc:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states
Below OID request sequence is validation:
OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT
^ |
| V
OID_SWITCH_NIC_CREATE <- OID_SWITCH_NIC_DELETE
In above sequence, the windows extensible switch interface assumes the
OID_SWITCH_PORT_CREATE has issued and the port has been created
successfully. If delete the internal port in HvDisconnectNic(),
HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because
there is no corresponding port.
Signed-off-by: Jinjun Gao
---
datapath-windows/ovsext/Vport.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 57e7510..9f1587f 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -628,6 +628,7 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
event.upcallPid = vport->upcallPid;
RtlCopyMemory(, >ovsName, sizeof event.ovsName);
event.type = OVS_EVENT_LINK_DOWN;
+OvsPostVportEvent();
/*
* Delete the port from the hash tables accessible to userspace. After this
@@ -635,13 +636,18 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
*/
if (OvsIsRealExternalVport(vport)) {
OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
-OvsPostVportEvent();
}
if (isInternalPort) {
OvsUnBindVportWithIpHelper(vport, switchContext);
-OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
-OvsPostVportEvent();
+/*
+ * Don't delete the port from the hash tables here for internal port
+ * because the internal port cannot be recreated in HvCreateNic(). It
+ * only can be created in HvCreatePort() by issuing
+ * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface
+ * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to
+ * delete the internal port.
+ */
}
NdisReleaseRWLock(switchContext->dispatchLock, );
--
1.8.5.6
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action
On 12/5/2019 6:46 PM, Sriharsha Basavapatna wrote:
> On Wed, Dec 4, 2019 at 8:55 PM Eli Britstein wrote:
>>
>> On 12/3/2019 5:19 PM, Sriharsha Basavapatna wrote:
>>> On Wed, Nov 20, 2019 at 9:07 PM Eli Britstein wrote:
Signed-off-by: Eli Britstein
Reviewed-by: Oz Shlomo
---
NEWS | 2 +
lib/netdev-offload-dpdk-flow.c | 87
--
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index 88b818948..ca9c2b230 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@ Post-v2.12.0
if supported by libbpf.
* Add option to enable, disable and query TCP sequence checking in
conntrack.
+ - DPDK:
+ * Add hardware offload support for output actions.
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/netdev-offload-dpdk-flow.c
b/lib/netdev-offload-dpdk-flow.c
index dbbc45e99..6e7efb315 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
rte_flow_action *actions)
} else {
ds_put_cstr(s, " RSS = null\n");
}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+ " Count: shared=%d, id=%d\n",
+ count->shared, count->id);
+} else {
+ds_put_cstr(s, " Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+ " Port-id: original=%d, id=%d\n",
+ port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, " Port-id = null\n");
+}
} else {
ds_put_format(s, "unknown rte flow action (%d)\n",
actions->type);
}
@@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
*patterns,
return 0;
}
+static void
+netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static void
+netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
+struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+
+port_id->id = netdev_dpdk_get_port_id(outdev);
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+}
+
+static int
+netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
+ const struct nlattr *nla,
+ struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (!netdev_dpdk_flow_api_supported(outdev)) {
+VLOG_DBG_RL(_rl,
+"Output to %s cannot be offloaded",
+netdev_get_name(outdev));
+return -1;
+}
+
+netdev_dpdk_flow_add_count_action(actions);
+netdev_dpdk_flow_add_port_id_action(actions, outdev);
+netdev_close(outdev);
+
+return 0;
+}
+
int
netdev_dpdk_flow_actions_add(struct flow_actions *actions,
struct nlattr *nl_actions,
size_t nl_actions_len,
- struct offload_info *info OVS_UNUSED)
+ struct offload_info *info)
{
struct nlattr *nla;
size_t left;
NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
-VLOG_DBG_RL(_rl,
-"Unsupported action type %d", nl_attr_type(nla));
-return -1;
+
Re: [ovs-dev] [PATCHv4] userspace: Add GTP-U support.
Hi William, GTP-U header size is not constant, you *must* take into account the flags, mainly the sequence. The use of sequence in GTP-U is optional but some devices do use it. see from 3GPP definition: "For PGW, SGW and eNodeB the usage of sequence numbers in G-PDUs is optional, but if GTP-U protocol entities in these nodes are relaying G-PDUs to other nodes, they shall relay the sequence numbers as well. An RNC, SGSN or GGSN shall reorder out of sequence T-PDUs when in sequence delivery is required. This is optional" The assumption that GTP-U is only data is not correct, you have some signaling for example END MARKER (like you wrote). This message is sent when there is a mobility between cells, and the message contains a TEID and indicates that last packet sent from the origin cell, to prevent packet re-order as handover might have packets on the fly. So I would expected that END MARKER will do the exact same forwarding as the GTP-U data. see inline >-Original Message- >From: dev On Behalf Of William Tu >Sent: Thursday, December 5, 2019 10:44 PM >To: d...@openvswitch.org >Subject: [ovs-dev] [PATCHv4] userspace: Add GTP-U support. > >GTP, GPRS Tunneling Protocol, is a group of IP-based communications >protocols used to carry general packet radio service (GPRS) within >GSM, UMTS and LTE networks. GTP protocol has two parts: Signalling >(GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used >for setting up GTP-U protocol, which is an IP-in-UDP tunneling >protocol. Usually GTP is used in connecting between base station for >radio, Serving Gateway (S-GW), and PDN Gateway (P-GW). > >This patch implements GTP-U protocol for userspace datapath, >supporting only required header fields and G-PDU message type. >See spec in: >https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftools.ietf.o >rg%2Fhtml%2Fdraft-hmm-dmm-5g-uplane-analysis- >00data=02%7C01%7Croniba%40mellanox.com%7Cb73529a0bea34637f9ed0 >8d779c3f1e4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63711175479 >1292921sdata=tY0GruNTBV9NWZg9gO2Lo4PWQZ1swHkB1AwdEg3AJUE%3 >Dreserved=0 > >Signed-off-by: Yi Yang >Co-authored-by: Yi Yang >Signed-off-by: William Tu >--- >v3 -> v4: > - applied Ben's doc revise > - increment FLOW_WC_SEQ to 42 > - minor fixes > - travis: >https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis- >ci.org%2Fwilliamtu%2Fovs- >travis%2Fbuilds%2F621289678data=02%7C01%7Croniba%40mellanox.com% >7Cb73529a0bea34637f9ed08d779c3f1e4%7Ca652971c7d2e4d9ba6a4d149256f461b >%7C0%7C0%7C637111754791292921sdata=eYRkIjcPUQqJwBQndsjCgtcPGG1 >l4QFUnAWW4vwPjpI%3Dreserved=0 > >v2 -> v3: > - pick up the code from v2, rebase to master > - many fixes in code, docs, and add more tests > - travis: >https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis- >ci.org%2Fwilliamtu%2Fovs- >travis%2Fbuilds%2F619799361data=02%7C01%7Croniba%40mellanox.com% >7Cb73529a0bea34637f9ed08d779c3f1e4%7Ca652971c7d2e4d9ba6a4d149256f461b >%7C0%7C0%7C637111754791292921sdata=cu5bdmJ4ydInKt4chxOcGy33K5E >9BMDzxgk5%2F7Cq%2FtI%3Dreserved=0 > >v1 -> v2: > - Add new packet_type PT_GTPU_MSG to handle GTP-U signaling message, >GTP-U signaling message should be steered into a control plane handler >by explicit openflow entry in flow table. > - Fix gtpu_flags and gptu_msgtype set issue. > - Verify communication with kernel GTP implementation from Jiannan >Ouyang. > - Fix unit test issue and make sure all the unit tests can pass. > - This patch series add GTP-U tunnel support in DPDK userspace, >GTP-U is used for carrying user data within the GPRS core network and >between the radio access network and the core network. The user data >transported can be packets in any of IPv4, IPv6, or PPP formats. >--- > Documentation/faq/configuration.rst | 13 +++ > Documentation/faq/releases.rst| 1 + > NEWS | 3 + > datapath/linux/compat/include/linux/openvswitch.h | 2 + > include/openvswitch/flow.h| 4 +- > include/openvswitch/match.h | 6 ++ > include/openvswitch/meta-flow.h | 28 ++ > include/openvswitch/packets.h | 4 +- > lib/dpif-netlink-rtnl.c | 5 + > lib/dpif-netlink.c| 5 + > lib/flow.c| 22 +++-- > lib/flow.h| 2 +- > lib/match.c | 30 +- > lib/meta-flow.c | 38 > lib/meta-flow.xml | 79 ++- > lib/netdev-native-tnl.c | 85 > lib/netdev-native-tnl.h | 8 ++ > lib/netdev-vport.c| 25 - > lib/nx-match.c
