On Tue, Jan 11, 2022 at 01:00:25PM +0100, Morten Brørup wrote:
[...]
> Also this version of the patch:
>
> Reviewed-by: Morten Brørup
My apologies Morten, don't know how I missed your tag.
With regards
Andrzej Ostruszka
Thank you Morten, Olivier and Konstantin for taking look at it.
I've just sent another version, with updates in commit message suggested
by Olivier.
With regards
Andrzej Ostruszka
efore wrapping to its beginning.
Fixes: cc4b218790f6 ("ring: support configurable element size")
Fixes: 286bd05bf70d ("ring: optimisations")
Signed-off-by: Andrzej Ostruszka
Reviewed-by: Olivier Matz
Acked-by: Konstantin Ananyev
---
lib/ring/rte_ring_elem_pvt.h | 12 ++--
fore wrapping to its beginning.
Signed-off-by: Andrzej Ostruszka
---
lib/ring/rte_ring_elem_pvt.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/ring/rte_ring_elem_pvt.h b/lib/ring/rte_ring_elem_pvt.h
index 275ec55393..83788c56e6 100644
--- a/lib/
this.
Wit regards
Andrzej Ostruszka
Andrzej Ostruszka (1):
ring: fix off by 1 mistake
lib/ring/rte_ring_elem_pvt.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
--
2.34.1.448.ga2b2bfdf31-goog
ction,
>> so Andrzej can rework while being sure his work will be accepted.
>>
>> Does it make sense?
>
> Make sense to me.
>
> I will leave @Andrzej Ostruszka to share his preference.
Yes this is fine with me, I'd be happy to participate in next meeting.
With regards
Andrzej
fine with adapting to any
standard of notification/communication that DPDK will agree on.
In addition may I ask your opinion on the changes that are required
before the library can be accepted?
Thank you in advance
With regards
Andrzej Ostruszka
y/port related, otherwise pass
>> NULL
>> + */
>> +void ifpx_notify_event(struct rte_ifpx_event *ev, struct ifpx_proxy_node
>> *px);
>> +
Will do - thank you.
Thank you for taking time to look at this.
With regards
Andrzej Ostruszka
which
does not happen. The packet will be sent to the kernel but it might
drop it, please check /proc/sys/net/ipv4/conf/dtap0/arp_accept and
related configuration options ('dtap0' here is just a name of your proxy
port).
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS
.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 3 +
config/common_base | 5 +
config/common_linux | 1 +
lib/Makefile | 3 +
lib/librte_eal/include/rte_eal_interrupts.h
This commit adds simple test of the library notifications.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 1 +
app/test/Makefile| 5 +
app/test/meson.build | 4 +
app/test/test_if_proxy.c | 707 +++
4 files changed, 717
but other are accepted in "round robin" fashion, or
some other logic).
As mentioned above Linux implementation is based on netlink socket.
This socket is registered as file descriptor in EAL interrupts
(similarly to how EAL alarms are implemented).
With regards
Andrzej Ostruszka
Andrzej Ost
This commit adds documentation of IF Proxy library.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS| 1 +
doc/guides/prog_guide/if_proxy_lib.rst | 142 +
doc/guides/prog_guide/index.rst| 1 +
3 files changed, 144 insertions
index 43f4c6337..277a6f109 100644
> --- a/doc/guides/prog_guide/lto.rst
> +++ b/doc/guides/prog_guide/lto.rst
> @@ -31,7 +31,7 @@ the whole DPDK by setting:
>
> .. code-block:: console
>
> -CONFIG_ENABLE_LTO=y
> +CONFIG_RTE_ENABLE_LTO=y
>
> in c
which
does not happen. The packet will be sent to the kernel but it might
drop it, please check /proc/sys/net/ipv4/conf/dtap0/arp_accept and
related configuration options ('dtap0' here is just a name of your proxy
port).
Signed-off-by: Andrzej Ostruszka
Depends-on: series-8862
-
This commit adds simple test of the library notifications.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 1 +
app/test/Makefile| 5 +
app/test/meson.build | 4 +
app/test/test_if_proxy.c | 707 +++
4 files changed, 717
.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 3 +
config/common_base | 5 +
config/common_linux | 1 +
lib/Makefile | 2 +
lib/librte_eal/include/rte_eal_interrupts.h
to all but other are accepted in "round robin" fashion, or
some other logic).
As mentioned above Linux implementation is based on netlink socket.
This socket is registered as file descriptor in EAL interrupts
(similarly to how EAL alarms are implemented).
With regards
Andrzej Ostrus
This commit adds documentation of IF Proxy library.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS| 1 +
doc/guides/prog_guide/if_proxy_lib.rst | 142 +
doc/guides/prog_guide/index.rst| 1 +
3 files changed, 144 insertions
at the
culprit is that at the end of rte_eth_iterator_init() there is a call:
iter->cls = rte_class_find_by_name("eth");
which was failing (because constructor registering eth class was not
being called).
This patch solves this issue.
Tested-by: Andrzej Ostruszka
With regards
Andrzej Ostruszka
these macros and libmnl when
it comes to handling of messages/attributes, so I decided to go with
macros instead of introducing external dependency. However I'm open to
change if that would be the general consensus.
With regards
Andrzej Ostruszka
---
[...]
> +#else
> +/*
> + * External ACK support was added in Linux kernel 4.17
> + * on older kernels, just ignore that part of message
> + */
> +#define tap_nl_dump_ext_ack(nh, err) do { } while();
Maybe "while(0)" here?
With regards
Andrzej Ostruszka
uint16_t port1;
> uint16_t port2;
> };
Initially it was so, but I made a comment that this leads to code
duplication in check_port_pair_config() (same checks for port1 and port2
- now handled via loop). I still stand by this comment :)
With regards
Andrzej Ostruszka
s should be obtained - and when this procedure completes a
* RTE_IFPX_CFG_DONE event should be signaled via
* ifpx_notify_event().
*/
extern
struct ifpx_platform_callbacks {
void (*init)(void);
uint64_t (*events)(void);
int (*listen)(void);
int (*close)(void);
void (*get_info)(int if_index);
} ifpx_platform;
With regards
Andrzej Ostruszka
ort/interface, which could be used not only by DPDK but by other too.
The answer from me is: no I have not. For two reasons:
- that would be Linux only
- if we would create such proxy, we would probably end up with tap like
driver in the end
With regards
Andrzej Ostruszka
sizeof(struct rte_lpm6_tbl_entry));
Acked-by: Andrzej Ostruszka
On 4/10/20 4:41 PM, Nithin Dabilpuram wrote:
> On Fri, Apr 10, 2020 at 03:50:06PM +0530, Nithin Dabilpuram wrote:
>> On Fri, Apr 10, 2020 at 01:07:34AM +0200, Andrzej Ostruszka wrote:
[...]
>>>> + next_hop = (rc == 0) ? next_hop : drop_nh;
>>>
>>> M
On 4/10/20 7:09 AM, Nithin Dabilpuram wrote:
> On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
[...]
>>> +struct rte_node_mbuf_priv2 {
>>> + union {
>>> + /* Sym crypto */
>>> + struct {
>>
.
I have finished the review and have no more comments ... apart from that
I like it :).
With regards
Andrzej Ostruszka
gt; + }
> + }
> +
> + /* !!! Home run !!! */
> + if (likely(last_spec == nb_objs)) {
> + rte_node_next_stream_move(graph, node, next_index);
> + return nb_objs;
> + }
> + held += last_spec;
> + rte_memcpy(to_next, from, last_spec * sizeof(from[0]));
> + rte_node_next_stream_put(graph, node, next_index, held);
OK. Forget my comments in different mail about difference between
encode/put - I got it now.
> +
> + return nb_objs;
> +}
> +
[...]
With regards
Andrzej Ostruszka
d from rte_graph_lookup().
> + * @param node
> + * Current node pointer.
> + * @param next
> + * Relative next node index..
> + * @param idx
> + * Number of objs updated in the stream after getting the stream using
> + * rte_node_next_stream_get.
> + *
> + * @see rte_node_next_stream_get().
> + */
> +__rte_experimental
> +static inline void
> +rte_node_next_stream_put(struct rte_graph *graph, struct rte_node *node,
> + rte_edge_t next, uint16_t idx)
I understood the stream_move as an optimized enqueue, but could you
describe how these _get/put are meant to be used, and why not use
stream_move/enqueue? I see in later (testing) patches that it is used
for source nodes, and I'm not sure I understand the difference between them.
With regards
Andrzej Ostruszka
I may change without prior notice
> + *
> + * Node mbuf private data to store crypto operation.
> + */
> +struct rte_node_mbuf_priv2 {
> + union {
> + /* Sym crypto */
> + struct {
> + struct rte_crypto_op op;
> + };
> + };
> +} __rte_cache_aligned;
Why such definition?
With regards
Andrzej Ostruszka
/* Extract ptype of mbuf0 */
> + eth_hdr = rte_pktmbuf_mtod(mbuf0, struct rte_ether_hdr *);
> + etype = eth_hdr->ether_type;
> + mbuf0->packet_type = l3_ptype(etype, 0);
> + }
> +
> + return nb_pkts;
> +}
[...]
With regards
Andrzej Ostruszka
(uint8_t)int_fld[FLD_PORT];
> + lcore_params_array[nb_lcore_params].queue_id =
> + (uint8_t)int_fld[FLD_QUEUE];
> + lcore_params_array[nb_lcore_params].lcore_id =
> + (uint8_t)int_fld[FLD_LCORE];
> + ++nb_lcore_params;
> + }
> + lcore_params = lcore_params_array;
> +
> + return 0;
> +}
With regards
Andrzej Ostruszka
.lpm_tbl[socket])
> + continue;
> +
> + ret = rte_lpm_add(ip4_lookup_nm.lpm_tbl[socket], ip, depth,
> + val);
> +
> + if (ret < 0) {
> + node_err("ip4_lookup",
> + "Unable to add entry %s / %d nh (%x) to LPM
> table on sock %d, rc=%d\n",
> + abuf, depth, val, socket, ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
With regards
Andrzej Ostruszka
maybe instead of another object
just add comment to route_add() below? No strong feeling here so you
can ignore this comment.
> +
> + in.s_addr = htonl(ipv4_l3fwd_lpm_route_array[i].ip);
> + snprintf(route_str, sizeof(route_str), "%s / %d (%d)",
> + inet_ntop(AF_INET, &in, abuf, sizeof(abuf)),
> + ipv4_l3fwd_lpm_route_array[i].depth,
> + ipv4_l3fwd_lpm_route_array[i].if_out);
> +
> + ret = rte_node_ip4_route_add(
> + ipv4_l3fwd_lpm_route_array[i].ip,
> + ipv4_l3fwd_lpm_route_array[i].depth, next_hop,
> + RTE_NODE_IP4_LOOKUP_NEXT_REWRITE);
> +
With regards
Andrzej Ostruszka
with PF and VF, certain functions works and other
> do not. In case of TUN PMD set/get mac_addr is not present.
TUN is not being used (for that reason) - only TAP. I could add check
for PRIMARY, but that way I would be artificially excluding cases where
that would work without the change. So for now I intend to leave things
like they are and address the actual problem (if it pops up). Note also
that I'm not checking errors for the mac_get/set so if given
functionality is not supported nothing will happen.
With regards
Andrzej Ostruszka
)
> + return graph;
> +
> + return graph_mem_fixup_node_ctx(graph);
> +}
> +
> +struct rte_graph *
> +rte_graph_lookup(const char *name)
> +{
> + const struct rte_memzone *mz;
> + struct rte_graph *rc = NULL;
> +
> + mz = rte_memzone_lookup(name);
> + if (mz)
> + rc = mz->addr;
> +
> + return graph_mem_fixup_secondray(rc);
> +}
[...]
With regards
Andrzej Ostruszka
foreach_node(count, off, graph, node) {
> + for (val = 0; val < node->nb_edges; val++) {
> + name = (const char *)node->nodes[val];
> + node->nodes[val] = graph_node_name_to_ptr(graph, name);
... Is it so that during node the first loop above some node might refer
(by name) to other node that is not yet "registered" so instead of
storing rte_node pointer you stored actually pointer to name which you
now update to proper rte_node?
> + if (node->nodes[val] == NULL)
> + SET_ERR_JMP(EINVAL, fail, "%s not found", name);
> + }
> + }
> +
> + return 0;
> +fail:
> + return -rte_errno;
> +}
[...]
With regards
Andrzej Ostruszka
raph))
> + goto graph_mem_destroy;
> +
> + /* All good, Lets add the graph to the list */
> + graph_id++;
> + STAILQ_INSERT_TAIL(&graph_list, graph, next);
> +
> + graph_spinlock_unlock();
> + return graph->id;
> +
> +graph_mem_destroy:
> + graph_fp_mem_destroy(graph);
> +graph_cleanup:
> + graph_cleanup(graph);
> +free:
> + free(graph);
> +fail:
> + graph_spinlock_unlock();
> + return RTE_GRAPH_ID_INVALID;
> +}
> +
With regards
Andrzej Ostruszka
On 4/7/20 2:27 PM, Jerin Jacob wrote:
> On Tue, Apr 7, 2020 at 5:46 PM Andrzej Ostruszka wrote:
>
>>> +int
>>> +graph_bfs(struct graph *graph, struct graph_node *start)
>>> +{
>>> + struct graph_node **queue, *v, *tmp;
>>> +
On 4/7/20 2:09 PM, Jerin Jacob wrote:
> On Tue, Apr 7, 2020 at 5:20 PM Andrzej Ostruszka wrote:
>>
>> On 4/7/20 12:22 PM, Jerin Jacob wrote:
[...]
>>>>> +static void
>>>>> +node_scan_dump(FILE *f, rte_node_t id, bool all)
>>>>> +{
>
; + if (graph_bfs(graph, graph_node))
> + goto fail;
> + }
> + }
> +
> + STAILQ_FOREACH(graph_node, &graph->node_list, next)
> + if (graph_node->visited == false)
> + SET_ERR_JMP(EINVAL, fail, "Found isolated node %s",
> + graph_node->node->name);> +
> + return 0;
You don't want to clear visited because it will not be used or cleared
on next call?
With regards
Andrzej Ostruszka
On 4/7/20 12:22 PM, Jerin Jacob wrote:
> On Mon, Apr 6, 2020 at 11:47 PM Andrzej Ostruszka wrote:
>>
>> On 4/5/20 10:55 AM, jer...@marvell.com wrote:
>>> From: Jerin Jacob
>>>
>>> Adding node debug API implementation support to dump
>>&g
t_nodes[] list */
>>> + if (from > node->nb_edges) {
>>> + rte_errno = ENOMEM;
>>> + goto fail;
>>> + }
>>> +
>>> + /* Remove me from list */
>>> + STAILQ_REMOVE(&node_list, node, node, next);
>
> This is where we will remove the node first unconditionally. Later we update
> the new node.
Thanks Kiran, don't know how I missed that :)
With regards
Andrzej Ostruszka
node_dump(f, node);
> + return;
> + }
> + }
> +fail:
> + return;
> +}
> +
> +void
> +rte_node_dump(FILE *f, rte_node_t id)
> +{
> + node_scan_dump(f, id, false);
> +}
> +
> +void
> +rte_node_list_dump(FILE *f)
> +{
> + node_scan_dump(f, 0, true);
> +}
> +
[...]
With regards
Andrzej Ostruszka
_CHECK(id);
> + graph_spinlock_lock();
> +
> + STAILQ_FOREACH(node, &node_list, next) {
> + if (node->id == id) {
> + if (next_nodes == NULL)
> + rc = sizeof(char *) * node->nb_edges;
> + else
> + rc = node_copy_edges(node, next_nodes);
Do we want to ready for next_nodes not large enough?
> + break;
> + }
> + }
> +
> + graph_spinlock_unlock();
> +fail:
> + return rc;
> +}
[...]
With regards
Andrzej Ostruszka
On 4/6/20 4:59 PM, Jerin Jacob wrote:
> On Mon, Apr 6, 2020 at 6:06 PM Andrzej Ostruszka wrote:
[...]
>>> +typedef uint32_t rte_graph_off_t; /**< Graph offset type. */
>>> +typedef uint32_t rte_node_t; /**< Node id type. */
>>> +typedef uint16_t
param size
> + * New size to shrink the edges.
> + *
> + * @return
> + * New size on success, RTE_EDGE_ID_INVALID otherwise.
> + */
> +__rte_experimental
> +rte_edge_t rte_node_edge_shrink(rte_node_t id, rte_edge_t size);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Get the edge names from a given node.
> + *
> + * @param id
> + * Valid node id.
> + * @param[out] next_nodes
> + * Buffer to copy the edge names. The NULL value is allowed in that case,
> + * the function returns the size of the array that needs to be allocated.
> + *
> + * @return
> + * When next_nodes == NULL, it returns the size of the array else
> + * number of item copied.
> + */
> +__rte_experimental
> +rte_node_t rte_node_edge_get(rte_node_t id, char *next_nodes[]);
I guess this doesn't copy names just stores pointers to names.
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Get maximum nodes created so far.
> + *
> + * @return
> + * Maximum nodes count on success, 0 otherwise.
> + */
> +__rte_experimental
> +rte_node_t rte_node_max_count(void);
If this is "created so far" then why call it 'max'? I guess this is max
possible so I would update description.
[...]
With regards
Andrzej Ostruszka
condition.
>
> doc/guides/rel_notes/release_20_05.rst | 6 +
> .../sample_app_ug/l2_forward_real_virtual.rst | 18 ++-
> examples/l2fwd/main.c | 169
> ++---
> 3 files changed, 172 insertions(+), 21 deletions(-)
Acked-by: Andrzej Ostruszka
Thomas,
I have replied to the other mail, here I just want to confirm, that I'm
fine with the proposed "general messaging" which other libraries (IF
Proxy including) could utilize.
See also below.
On 4/3/20 7:19 PM, Thomas Monjalon wrote:
> 02/04/2020 15:48, Andrzej Ostruszka
te terms I might be
switching between general comments and specific examples.
On 4/3/20 11:42 PM, Thomas Monjalon wrote:
> Hi Andrzej,
[...]
> 10/03/2020 12:10, Andrzej Ostruszka:
[...]
>> The purpose of this library is to help with both of these tasks (as long
>> as they remai
hat can be
set to FF and use memset() and for the remaining user rte_bitmap_set()?
Right now you are also touching array2 memory which was already set above.
> + /* Fill the arry2 left not byte aligned bits. */
> + for (i = slabs * RTE_BITMAP_SLAB_BIT_SIZE; i < n_bits; i++)
> + rte_bitmap_set(bmp, i);
> + return bmp;
> +}
> +
With regards
Andrzej Ostruszka
re enabled
> before any hardware configurations initiated on the allocated nix lf.
>
> Signed-off-by: Vamsi Attunuru
> ---
Acked-by: Andrzej Ostruszka
printf("Notice: odd number of ports in portmask.\n");
> + l2fwd_dst_ports[last_port] = last_port;
> }
> - else
> - last_port = portid;
> -
> - nb_ports_in_mask++;
> - }
> - if (nb_ports_in_mask % 2) {
> - printf("Notice: odd number of ports in portmask.\n");
> - l2fwd_dst_ports[last_port] = last_port;
> }
>
> rx_lcore_id = 0;
> @@ -613,7 +748,8 @@ main(int argc, char **argv)
>
> qconf->rx_port_list[qconf->n_rx_port] = portid;
> qconf->n_rx_port++;
> - printf("Lcore %u: RX port %u\n", rx_lcore_id, portid);
> + printf("Lcore %u: RX port %u TX port %u\n", rx_lcore_id,
> +portid, l2fwd_dst_ports[portid]);
> }
>
> nb_mbufs = RTE_MAX(nb_ports * (nb_rxd + nb_txd + MAX_PKT_BURST +
>
Apart from these comments:
Reviewed-by: Andrzej Ostruszka
With regards
Andrzej Ostruszka
On 3/26/20 6:42 PM, Andrzej Ostruszka wrote:
> On 3/25/20 12:11 PM, Morten Brørup wrote:
[...]
>> And I am still strongly opposed to the callback method:
>
> Noted - however for now I would like to keep them. I don't have much
> experience with this library so if they prov
First of all thank you Vipin for taking a look at this.
On 4/1/20 7:29 AM, Varghese, Vipin wrote:
> snipped
>> diff --git a/lib/librte_if_proxy/Makefile b/lib/librte_if_proxy/Makefile
>> new file mode 100644
>> index 0..43cb702a2
>> --- /dev/null
>> +++ b/lib/librte_if_proxy/Makefile
>> @@
(Andrzej)
>
> v2 Changes:
> - Fix minor formatting error.
> - Change uint8_t to bool.
Reviewed-by: Andrzej Ostruszka
On 3/31/20 2:36 PM, Harman Kalra wrote:
> On Fri, Mar 06, 2020 at 05:41:01PM +0100, Andrzej Ostruszka wrote:
[...]
>> +void ifpx_notify_event(struct rte_ifpx_event *ev, struct ifpx_proxy_node
>> *px)
>> +{
>> +struct ifpx_queue_node *q;
>> +int done
printf("port %u is used in other port pairs\n", portid);
> + return -1;
> + }
> +
> + port_pair_mask |= (1 << portid);
> + port_pair_mask |= (1 << index);
> + }
In the above loop you are doing checks twice. Suppose you have pair
(2,3) and you go by index from 0 (like you do) and reach point i=2.
Then you check i=2 and p=3, then on next iteration you do the same
checks (this time i=3,p=2). I guess simple fix would be to skip loop
iteration both on not enabled (like you do) and on check if the port was
already checked (test bit in port_pair_mask).
With regards
Andrzej Ostruszka
On 3/26/20 1:41 PM, Andrzej Ostruszka wrote:
> Thank you David for taking time to look at this.
>
> On 3/25/20 9:08 AM, David Marchand wrote:
>> Hello Andrzej,
>>
>> On Tue, Mar 10, 2020 at 12:11 PM Andrzej Ostruszka
> [...]
>> I can see we end up exposing
t;
> Example:
> -w 0002:02:00.0,lock_rx_ctx=1 -w 0002:03:00.0,lock_tx_ctx=1
>
> Signed-off-by: Pavan Nikhilesh
> Reviewed-by: Andrzej Ostruszka
> ---
> Depends on http://patches.dpdk.org/patch/67178/
> v4 Changes:
> - Fix return path using unnecessary goto.(Andrz
d by a another Linux process?
I believe so. However there is a big question whether my assumption
about LPM is correct. I've looked at the code and it looks like it so
but I'm not in power to authoritatively declare it. So again, to me LPM
looks like safe to be changed by a single writer while being used by
multiple readers (with an obvious transient period when rule is being
expanded and some IPs might go with an old and some with a new destination).
With regards
Andrzej Ostruszka
his is
because you call parse_flag() which is treating its extra_arg as pointer
to uint16_t.
> if (devargs == NULL)
> goto null_devargs;
> @@ -161,6 +165,10 @@ otx2_ethdev_parse_devargs(struct rte_devargs *devargs,
> struct otx2_eth_dev *dev)
> &parse_switch_header_type, &switch_header_type);
> rte_kvargs_process(kvlist, OTX2_RSS_TAG_AS_XOR,
> &parse_flag, &rss_tag_as_xor);
> + rte_kvargs_process(kvlist, OTX2_LOCK_RX_CTX,
> +&parse_flag, &lock_rx_ctx);
> + rte_kvargs_process(kvlist, OTX2_LOCK_TX_CTX,
> +&parse_flag, &lock_tx_ctx);
> otx2_parse_common_devargs(kvlist);
> rte_kvargs_free(kvlist);
[...]
With that uint16_t fix above:
Reviewed-by: Andrzej Ostruszka
With regards
Andrzej Ostruszka
lib/librte_flow_classify/rte_flow_classify.c | 7 ++-
> 8 files changed, 43 insertions(+), 21 deletions(-)
Ferruh
I like this and would like use it for the next round of my patches. Is
there any timeline for that?
With regards
Andrzej Ostruszka
Reviewed-by: Andrzej Ostruszka
Thank you David for taking time to look at this.
On 3/25/20 9:08 AM, David Marchand wrote:
> Hello Andrzej,
>
> On Tue, Mar 10, 2020 at 12:11 PM Andrzej Ostruszka
[...]
> I can see we end up exposing structures for registering callbacks.
Right. I was thinking more in terms of user
tsc); \
}} while(0)
That way handling of RX and TX would be very symmetrical - you'd just
call TX_ADD/RX_ADD where needed and you'd remove both "total TX bump"
and "total RX bump" code.
Note however that accepting this suggestion and removal of "total RX
bump" would also be a behaviour change - since currently when no packet
is read nothing happens (apart from the tics updated) whereas with the
proposed changes (and enabled burst stats) you'd be bumping
rx_burst_stats.pkt_burst_spread[0]. I'd say that would be change for
good since this would allow to have a glimpse how many iterations are idle.
> #else
> /* No profiling statistics is configured. */
> #define TEST_PMD_CORE_CYC_TX_START(a)
> #define TEST_PMD_CORE_CYC_RX_START(a)
> #define TEST_PMD_CORE_CYC_FWD_ADD(fs, s)
> -#define TEST_PMD_CORE_CYC_TX_ADD(fs, s)
> -#define TEST_PMD_CORE_CYC_RX_ADD(fs, s)
> +#define TEST_PMD_CORE_CYC_TX_ADD(fs, s, np)
> +#define TEST_PMD_CORE_CYC_RX_ADD(fs, s, np)
> #endif /* RTE_TEST_PMD_RECORD_CORE_CYCLES */
[...]
With regards
Andrzej Ostruszka
Reviewed-by: Andrzej Ostruszka
t; + if (idev == NULL)
> + return 0;
> +
> + if (!(idev->npa_lock_mask & BIT_ULL(aura_id)))
> + return 0;
Same comments here and below as for *pool_init above.
> +
> + aura_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox);
> + aura_req->aura_id = aura_id;
> + aura_req->ctype = NPA_AQ_CTYPE_AURA;
> + aura_req->op = NPA_AQ_INSTOP_UNLOCK;
> +
> + rc = otx2_mbox_process(mbox);
> + if (rc < 0)
> + otx2_err("Failed to unlock AURA ctx to NDC");
> +
> + pool_req = otx2_mbox_alloc_msg_npa_aq_enq(mbox);
> + pool_req->aura_id = aura_id;
> + pool_req->ctype = NPA_AQ_CTYPE_POOL;
> + pool_req->op = NPA_AQ_INSTOP_UNLOCK;
> +
> + rc = otx2_mbox_process(mbox);
> + if (rc < 0)
> + otx2_err("Failed to unlock POOL ctx to NDC");
> +
> return 0;
> }
With regards
Andrzej Ostruszka
G_AS_XOR "=1");
> + OTX2_RSS_TAG_AS_XOR "=1"
> + OTX2_LOCK_RX_CTX "=<1-65535>"
> + OTX2_LOCK_TX_CTX "=<1-65535>");
AFAIU the "=1" is required due to kvargs parsing limitation. But why
the range here is 1-65535 when all you want is just a boolean flag (if
the key is present or not).
> diff --git a/drivers/net/octeontx2/otx2_rss.c
> b/drivers/net/octeontx2/otx2_rss.c
> index 7a8c8f3de..34005ef02 100644
> --- a/drivers/net/octeontx2/otx2_rss.c
> +++ b/drivers/net/octeontx2/otx2_rss.c
> @@ -33,6 +33,29 @@ otx2_nix_rss_tbl_init(struct otx2_eth_dev *dev,
> req->qidx = (group * rss->rss_size) + idx;
> req->ctype = NIX_AQ_CTYPE_RSS;
> req->op = NIX_AQ_INSTOP_INIT;
> +
> + if (!dev->lock_rx_ctx)
> + continue;
> +
> + req = otx2_mbox_alloc_msg_nix_aq_enq(mbox);
> + if (!req) {
> + /* The shared memory buffer can be full.
> + * Flush it and retry
> + */
> + otx2_mbox_msg_send(mbox, 0);
> + rc = otx2_mbox_wait_for_rsp(mbox, 0);
> + if (rc < 0)
> + return rc;
> +
> + req = otx2_mbox_alloc_msg_nix_aq_enq(mbox);
> + if (!req)
> + return -ENOMEM;
> + }
> + req->rss.rq = ind_tbl[idx];
> + /* Fill AQ info */
> + req->qidx = (group * rss->rss_size) + idx;
> + req->ctype = NIX_AQ_CTYPE_RSS;
> + req->op = NIX_AQ_INSTOP_LOCK;
> }
>
> otx2_mbox_msg_send(mbox, 0);
>
And here you treat the locking errors as errors - so I think you need to
just adapt to this style and fix the previous comments.
With regards
Andrzej Ostruszka
Nithin
On 3/12/20 12:18 PM, Nithin Dabilpuram wrote:
> Add support to traffic management api in OCTEON TX2 PMD.
> This support applies to CN96xx of C0 silicon version.
>
> This series depends on http://patchwork.dpdk.org/patch/66344/
>
> Depends-on:series-66344
If I understood the proposal corr
This commit adds simple test of the library notifications.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 1 +
app/test/Makefile| 5 +
app/test/meson.build | 4 +
app/test/test_if_proxy.c | 707 +++
4 files changed, 717
which
does not happen. The packet will be sent to the kernel but it might
drop it, please check /proc/sys/net/ipv4/conf/dtap0/arp_accept and
related configuration options ('dtap0' here is just a name of your proxy
port).
Signed-off-by: Andrzej Ostruszka
Depends-on: series-8862
-
.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 3 +
config/common_base| 5 +
config/common_linux | 1 +
lib/Makefile | 2 +
.../common/include/rte_eal_interrupts.h
This commit adds documentation of IF Proxy library.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS| 1 +
doc/guides/prog_guide/if_proxy_lib.rst | 142 +
doc/guides/prog_guide/index.rst| 1 +
3 files changed, 144 insertions
on and other
performance improvements) version of l3fwd that should serve as an
example of using this library in real applications.
Changes in V2
=
- Cleaned up checkpatch warnings
- Removed dead/unused code and added gateway clearing in l3fwd-ifpx
With regards
Andrzej Ostruszka
Note: Patch 4 in t
to have addresses passed as
const so I'm adding this fixline.
Fixes: d82927d2f81d ("lpm6: make IPv6 address immutable")
Cc: step...@networkplumber.org
Signed-off-by: Andrzej Ostruszka
---
V2 changes:
---
* reverted changes to the bulk functions and modified commit messa
On 3/9/20 8:13 PM, Stephen Hemminger wrote:
> On Mon, 9 Mar 2020 18:02:21 +
> "Medvedkin, Vladimir" wrote:
>
>> On 09/03/2020 15:52, Stephen Hemminger wrote:
>>> On Mon, 9 Mar 2020 13:38:53 +
>>> "Medvedkin, Vladimir" wrote:
>>>
Hi Andrze,
Adding const qualifier for bulk
My apologies - I have forgotten to run checkpatch on the series.
I will correct these in version 2 - in the mean time please skip these
minor faults and comment on the rest.
With regards
Andrzej Ostruszka
which
does not happen. The packet will be sent to the kernel but it might
drop it, please check /proc/sys/net/ipv4/conf/dtap0/arp_accept and
related configuration options ('dtap0' here is just a name of your proxy
port).
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS
This commit adds simple test of the library notifications.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 1 +
app/test/Makefile| 5 +
app/test/meson.build | 4 +
app/test/test_if_proxy.c | 706 +++
4 files changed, 716
This commit adds documentation of IF Proxy library.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS| 1 +
doc/guides/prog_guide/if_proxy_lib.rst | 142 +
doc/guides/prog_guide/index.rst| 1 +
3 files changed, 144 insertions
.
Signed-off-by: Andrzej Ostruszka
---
MAINTAINERS | 3 +
config/common_base| 5 +
config/common_linux | 1 +
lib/Makefile | 2 +
.../common/include/rte_eal_interrupts.h
hat to do with it. If
that is not desired and such packets should be dropped then null port
can be created with proxy and e.g. default route installed on it.
- Removed previous example which was just printing notification.
Instead added a simplified (stripped vectorization and other
perform
ber.org
Signed-off-by: Andrzej Ostruszka
---
lib/librte_lpm/rte_lpm6.c | 14 +++---
lib/librte_lpm/rte_lpm6.h | 13 +++--
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index d515600f1..6089217d6 100644
--- a
On 2/14/20 4:47 PM, Stephen Hemminger wrote:
> On Fri, 14 Feb 2020 15:37:59 +0100
> Andrzej Ostruszka wrote:
>
>> LPM6 does not modify input IPv6 addresses so mark them as 'const'.
>>
>> The real need is to add those annotations to the API but in additio
LPM6 does not modify input IPv6 addresses so mark them as 'const'.
The real need is to add those annotations to the API but in addition
they were added also to internal functions and slight formatting changes
were performed (aligning of input args).
Signed-off-by: Andrzej Ostruszka
Timer, LPM and Distributor libraries no longer use function versioning
and therefore do not need separate build for static and shared version
of libraries.
This patch removes use_function_versioning from their meson build files
and corresponding include from the sources.
Signed-off-by: Andrzej
ing
gcc bug - it does not have proper way to mark .symver in internal
representation and so does not recognize that function is used).
This comment is global - for all symbols mentioned in BIND_/VERSION_ macros.
Out of curiosity - why do you need separate declaration just before
definition?
With regards
Andrzej Ostruszka
On 1/29/20 5:25 PM, Ferruh Yigit wrote:
> On 1/29/2020 2:57 PM, Andrzej Ostruszka wrote:
[...]
>> I've now noticed that LPM, Distributor and Timer libraries no longer use
>> function versioning but have it still set in their meson.build.
>
> Right, I can remove them.
Timer, LPM and Distributor libraries no longer use function versioning
and therefore do not need separate build for static and shared version
of libraries.
This patch removes use_function_versioning from their meson build files.
Signed-off-by: Andrzej Ostruszka
Fixes: f2fb215843a9 ("
e them have it set to true (and perform
separate shared build - see lib/meson.build).
I've now noticed that LPM, Distributor and Timer libraries no longer use
function versioning but have it still set in their meson.build.
With regards
Andrzej Ostruszka
> meson -Ddisable_drivers=crypto/*,event/* build
Maybe documenting that in 'build-sdk-meson.txt' would be beneficial?
With regards
Andrzej Ostruszka
wrote:
>> -Original Message-
>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrzej Ostruszka
>> Sent: Thursday, January 16, 2020 11:43 AM
[...]
>> You'd still need to repack the message and that could be the job of the
>> callback.
>>
>>
acks is returned as a
bitmask. This API will change if we find out the need for more callbacks.
With regards
Andrzej Ostruszka
our depending on the proxy used.
But again this kind of uses are not what I expect as a common use case
so I went with current design.
> And on the administrative side, I assume one of you guys will volunteer
> as the maintainer of this library?
Yes.
With regards
Andrzej Ostruszka
On 1/14/20 4:16 PM, Morten Brørup wrote:
> Andrzej,
Hello Morten
> Basically you are adding a very small subset of the Linux IP stack> to
> interface with DPDK applications via callbacks.
Yes, at the moment this is limited - we'd prefer first to solicit
some input from community.
> The library
t 1 going up
route6 add -> ff00::/8
route6 add -> fe80::/64
address6 add for port 0 -> fe80::2436:17ff:fefd:94ed
address6 add for port 1 -> fe80::2436:17ff:fefd:94ed
route6 add -> fe80::2436:17ff:fefd:94ed/128
Signed-off-by: Andrzej Ostruszka
---
This commit adds a preliminary Linux implementation of the IF Proxy
library. It should allow one to play around with the idea and check its
usefulness.
Signed-off-by: Andrzej Ostruszka
---
config/common_base| 5 +
lib/Makefile
gathered is passed to the application via a set of user
registered callbacks. This way user can use normal network utilities
(like those from the iproute2 suite) to configure DPDK ports.
Signed-off-by: Andrzej Ostruszka
---
lib/librte_if_proxy/rte_if_proxy.h | 364
her community feedback
- polish the implementation:
* call the notification callbacks without lock held (at the moment
attempts to modify callbacks from within the callback would deadlock)
* separate the system dependent parts from the rest so that it is easy
to figure out what needs to be reim
1 - 100 of 307 matches
Mail list logo
