From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 96C4843E0E; Sat, 6 Apr 2024 01:11:34 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 702EF4028C; Sat, 6 Apr 2024 01:11:34 +0200 (CEST) Received: from mail-qt1-f181.google.com (mail-qt1-f181.google.com [209.85.160.181]) by mails.dpdk.org (Postfix) with ESMTP id 6B9D940262 for ; Sat, 6 Apr 2024 01:11:32 +0200 (CEST) Received: by mail-qt1-f181.google.com with SMTP id d75a77b69052e-432c947e92eso33796801cf.0 for ; Fri, 05 Apr 2024 16:11:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712358692; x=1712963492; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=hNj4NzOSNDRYuleoqDpDBEq99yKkZlhD2gdlZ2m417g=; b=JI/lyys+gGPerOHRLo4htuHdopQ1n9+5+hdsnKrI0x8Yc0akm26yVVkGtKsmP6buPE 8Nj758vltMq2zFqT7lSnIi6R15Xhiicg3wW1itel5pFLEwCrDZuyogetKWh2uiFajqyM G3k88P2eOA5N8H9VZw3rHOJZVm3TtIdjPHbS/Eoz77xQvY1e5msjTKPJgNqgrXEd3TOO nQSRLsrCtUrKA4EJVZyNsfWxos9LGTNxD6YFv1IkHGrBBRfY2XXcR3rDYdd3thnuFfEt fCCHsVNOf1LrsiXIARobHMVfYKczIlpK8y0eZKtfpvvNoKTOz857Fyouhd7+FOwqbGxV CzFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712358692; x=1712963492; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hNj4NzOSNDRYuleoqDpDBEq99yKkZlhD2gdlZ2m417g=; b=oFaUFCsl8ga3O5CWNuJkxmrZv5qU+ibuwxTH9PbtHWbGGUN8BDcIwsKj1qoEJBVTrF 8+ZqHJyHxmfOvRTpscz/0n4CTL1ZNA5+ijkzbqLVxZMqXxOgHNLe8sJ8dnLXYsYBxQQ+ lqta50f1IKCjIDxsn6eIBFWV60c3yQsMZQv/D2yyDX6At9SlgeNNn8PUpoWb3zfLG/sL FohubRYk/RXESgcVYYEMRx60mPD2Aa+n96izIPCl1WcY7avZQklpCXdOcMeukeXJLm5y B0ZCJ8lpjKrSgmbIG1FDDlMc4q68UmSuWOPDwsxZxbwGKQg0efGRDQeu2pc0X+9Gs1J2 nSmA== X-Gm-Message-State: AOJu0Yx0KO0JvAYb1Qps3wa6QAtZIM5sqRZYApQOabHKMWz7nbNv0k9i d0UWAxGOWZsWCedbQamrQQKRT+tVr1Q10VtyyndAxMcqXDGEHmimdyOpTExVj/bo/k4Ns37tYMJ e8XeitxXuffyl3b3GQqmDW9Jo9QA= X-Google-Smtp-Source: AGHT+IExpXvs4EjZaJsOw0yluODaX5xx5H0NQd4XIZXC0brkcalJz/CwMPknSCh4IJrOHdHEzzme0SCtuooaVo8W7pw= X-Received: by 2002:ac8:5715:0:b0:432:b560:41fa with SMTP id 21-20020ac85715000000b00432b56041famr5134577qtw.34.1712358691731; Fri, 05 Apr 2024 16:11:31 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jerin Jacob Date: Sat, 6 Apr 2024 04:41:05 +0530 Message-ID: Subject: Re: [RFC] graph/node: feedback and future improvements To: Robin Jarry Cc: dev@dpdk.org, Jerin Jacob , Kiran Kumar K , Nithin Dabilpuram , Pavan Nikhilesh , Hongbo Zheng , Zhirun Yan , Thomas Monjalon Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Mar 25, 2024 at 7:56=E2=80=AFPM Robin Jarry wro= te: > > Hi Jerin, all, > > I apologize in advance for the long email :) > I am working on a project [1] that uses rte_graph extensively. In the Great. You may consider improving and/or adding inbuilt nodes for generic protocol processing. Furthermore, consider contributing on app/graph. I think, most likely, you should be able to leverage app/graph. > course of action, I have stumbled upon a few issues. I managed to work > around them for now, but I'd like to get more insights about long term > solutions. > > Per Rx/Tx queue nodes > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > In the in-built nodes and in the examples, there is one ethdev_rx and > ethdev_tx node per rx/tx queue [2]. > > Is there a technical reason for this design? Does it make sense to have The default Rx/Tx nodes added to support l3fwd-graph example application. > only one of each ethdev_rx and ethdev_tx nodes per graph? For simplicity > and to make dynamic rxq changes possible, I chose to have a single rx > & tx node per graph. Do you think we could change the in-built nodes to > support both modes ? In terms of performance, the current scheme will be more performant. I would suggest, we can add another inbuilt node for this. This is to avoid additional checks in fast path to enable dynamic behavior. Probably need to use rcu as control thread updates port/queue configuration changes and fast path needs to adapt to it. > > Having multiple instances of the same node in a graph complicates > instantiation as it requires cloning the nodes with unique names. Also, > it makes dynamic configuration of ports even more complicated without > shutting down everything first since some nodes will be part of an > active graph and there may be conflicts. > > Speaking of graph reloading, I found that the in-built ethdev_tx TX > queue id is initialized to graph_id [3]. This seems odd. If there are > multiple rounds of graph create/destroy, the id will become invalid. > > Dynamic graph and nodes construction/destruction > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > I need to deal with reconfiguration of the graphs at runtime. This can > happen on multiple occasions: port addition/suppression, number of rxq > change, rxq size change, etc. > > I could not manage to reuse the in-built nodes because of the issues > raised in the previous point. Having a new inbuilt node as mentioned above will fix this issue. > > Could we change the in-built nodes to better support dynamic reloading? > Maybe this only applies to nodes that may appear multiple times in the > same graph (rx/tx). > > Node context data > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > There is no way to prepare node data context when calling > rte_graph_create(). The current implementation uses global variables [4] > but this makes it very "static". Since the those are node and it is private. I think it is OK. Also using, rte_graph_node_get_*() one can get the node and its ctx at anyt= ime. > > It would be nice to pass prepared context data for every node on graph > creation, either via a config parameter (better) or via another I think, rte_graph_create() will be complicated, e.s.p it supports loading = nodes with regex pattern. I think, we can weigh in pros and cons if you have patc= h. > mechanism. I currently do this via a hash map but it requires a global > hash as well which may not be the best solution. > > I tried patching the graph library code myself but after struggling, > I thought it would be best to discuss things first. > > Pluggable nodes > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > Currently, the declaration of next nodes is static. In order to support > plugins (e.g. via dlopen() or similar), could we introduce a way to > dynamically insert a node in the graph? > > I have done this using a post-init callback system but we could think of > a different way. > > Also, could we allow overriding nodes with RTE_NODE_REGISTER()? So users > can replace the default implementation with their own if they need to. I think, if we document the inbuilt node's downstream node ID. After rte_graph_create(), one can use rte_node_edge_update() to dynamically add custom/user defined n= odes in between. I thought of adding more helper functions (leveraging existing rte_node_edge_update() for this) It will be more like "feature arch" in VPP. Provided, node cannot add dynamically after rte_graph_create(), but one changes the downstream node connection dynamically. > > Move data pointer of packets? > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > This final point is more a global design question. In traditional > networking stacks, each layer of the stack receives pbuffers that are > "adjusted" to their respective network header in the packet (i.e. the IP > lookup function will receive a buffer that points to the beginning of > the IP header, regardless of what was the transport protocol, plain > ethernet, Ethernet + VLAN, IP in IP, etc.). > > Would it make sense to have a similar mechanism when designing an > application with rte_graph? Yes. I think, we can use mbuf data offset or new dynamic mbuf filed for thi= s. > > Thanks in advance for your replies. > > Cheers! > > -- > Robin > > [1] https://github.com/rjarry/brouter > [2] https://github.com/DPDK/dpdk/blob/v23.11/lib/node/ethdev_rx.c#L28 > [3] https://github.com/DPDK/dpdk/blob/v23.11/lib/node/ethdev_tx.c#L56 > [4] https://github.com/DPDK/dpdk/blob/v23.11/lib/node/ethdev_ctrl.c >