From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1CB4C45B5A; Thu, 17 Oct 2024 09:07:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A8BB740279; Thu, 17 Oct 2024 09:07:26 +0200 (CEST) Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by mails.dpdk.org (Postfix) with ESMTP id 1400C40150 for <dev@dpdk.org>; Wed, 16 Oct 2024 15:51:13 +0200 (CEST) Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2fabb837ddbso84742981fa.1 for <dev@dpdk.org>; Wed, 16 Oct 2024 06:51:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729086672; x=1729691472; 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=GKlKAvEtOOz0lPWSqjzo4QSlthlKC80p6s3ov+neJ6o=; b=BC/z8ssOON4mMBIFoisQRKmqRBVLiEhmDwZPzUIpzp0VV1p8rI43343uiCVxFxjPOy qcrbobHiVjkIQOoQYOb9zu8fiqphv9S77RYWIONVyL97zpuKZ+H5vwGIJ31cLrhToHx3 gmqPpNNVhMfzKOnUKHWTWniASiSTy3F6cJvwsTyg3Yv8VIcYUNOFXjbbk9ZTKOmx22bH cs1IOu0OSFzO/GLZLV86pDXFNpNIO02qzirPt65E7x28VG8IT3mBWPfzfGV0gJCxg1/u rVaTOcGmsBtAAKVMEgVB/Isw4l3vMk8tQtQh53p4xvcwRlut2lIxrqKALQ12zIKpx5+y v/Qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729086672; x=1729691472; 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=GKlKAvEtOOz0lPWSqjzo4QSlthlKC80p6s3ov+neJ6o=; b=bsddXWyiWJRA0HKLLoDFcdxjgpT6VttfJp+1eKD8UdIHCiMZIJOUcTNA1AWnGoePVC h5eeyV/V7rzIRBskgG0Kgcm4JlGjFaluOdAZ/q/wBW+w4pchBy1wVRT/oATph1JdEnWu Q3E/DOIGAztt5uFSpHjGJ5eRwbucLDvl0VOIBUD921cLMIx/iStYF57Ou3l7mM1iaERr pOc8yF9Od+In4unD0wcDyROS1HTDhO6X9ps4EzFtKhGAzYOTM6mM9GGM0IKYeCGzZwl8 liycXEN/RtRzt8vJ3DfqJieS0h+rt+LgnLqGIqjYusJTFGpy5vroLqCCSz2tt+f81/ZF ussg== X-Forwarded-Encrypted: i=1; AJvYcCX4EGEb8KD1oDiqA1gxkQ4Qq5MwJhuZvGLp8WSI2Sg7F+psanFJamqrQ0yylp9VOkATuJU=@dpdk.org X-Gm-Message-State: AOJu0YxkVS6IKuTi9yJErHlU51uS+WISb3sRM7CaG7tCye372bH6Gcw2 WU7HI0bQeRHYBb4fZqF2JBW7t3B1mn5F5IkDOKfd3n8wQRaJeznV24e2blSoK2/+1N9q7V2ljm7 UEt0T5H0+uuzKpAqN59L2EZ88Xds= X-Google-Smtp-Source: AGHT+IF+DoltRN4oexoGhT6HIJ+zopqWbCkqHXvBOTESLw9HS5jp+rk7BHAeKau5MNpgDSNrP0i5DGcgO5lKii7XkJ0= X-Received: by 2002:a2e:a584:0:b0:2fb:cc0:2a04 with SMTP id 38308e7fff4ca-2fb3f1d4a64mr84319521fa.25.1729086672002; Wed, 16 Oct 2024 06:51:12 -0700 (PDT) MIME-Version: 1.0 References: <20240907073115.1531070-1-nsaxena@marvell.com> <CAJFAV8zRkre6OhfcXrhEK8sCzZ7BB9q-foSXeJ_2c_xqcAq2mw@mail.gmail.com> <SJ0PR18MB511158FD7D8D0263533CAEDBB6442@SJ0PR18MB5111.namprd18.prod.outlook.com> <CAJFAV8wmEOu8e-2dTojYoZTb2SRjUitikGvi9tVMuuq-933Ejg@mail.gmail.com> <D4X4P8CDPZ5M.1WRRVPNRNKCHR@redhat.com> In-Reply-To: <D4X4P8CDPZ5M.1WRRVPNRNKCHR@redhat.com> From: Nitin Saxena <nsaxena16@gmail.com> Date: Wed, 16 Oct 2024 19:20:59 +0530 Message-ID: <CAG6-93zHXox3CRhC-QwsMM7FfK+5JgokjW+QinK99anaBU2djA@mail.gmail.com> Subject: Re: [EXTERNAL] Re: [RFC PATCH 0/3] add feature arc in rte_graph To: Robin Jarry <rjarry@redhat.com> Cc: David Marchand <david.marchand@redhat.com>, Nitin Saxena <nsaxena@marvell.com>, Jerin Jacob <jerinj@marvell.com>, Kiran Kumar Kokkilagadda <kirankumark@marvell.com>, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>, Zhirun Yan <yanzhirun_163@163.com>, "dev@dpdk.org" <dev@dpdk.org>, Christophe Fontaine <cfontain@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Thu, 17 Oct 2024 09:07:24 +0200 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Hi Robin, Thanks for the review Please see my replies inline Thanks, Nitin On Wed, Oct 16, 2024 at 3:08=E2=80=AFPM Robin Jarry <rjarry@redhat.com> wro= te: > > Hi folks, > > David Marchand, Oct 16, 2024 at 11:24: > > On Mon, Oct 14, 2024 at 1:12=E2=80=AFPM Nitin Saxena <nsaxena@marvell.c= om> wrote: > >> I had pushed non RFC patch series before -rc1 date (11th oct). > >> We have an ABI change in this patch series https://patches.dpdk.org/pr= oject/dpdk/patch/20241010133111.2764712-3-nsaxena@marvell.com/ > >> Could you help merge this patch series in rc2 otherwise it has to wait= for next LTS > > > > Just read through the series, I am not confident with this addition. > > It requires a lot of changes in the node code for supporting it, where > > it should be something handled in/facilitated by the graph library > > itself. > > As far as I can tell, it will be very complicated (if not impossible) to > determine in a generic manner whether a packet must be steered towards > a sub tree or not. The decision *must* come from the originating node in > some way or another. Nitin> I am not sure if it *must* always be from the originating node? What about a control plane which wants to enable "IP4 feature" on interface 'X' by assigning IP address? A originating node (say: ip4-input) *must not* activate IP4 lookup sub-graph for interface "X " until control plane assigns any IP address to it. Regarding the complexity of adopting feature arc changes in fast path, - a sub-optimal change for feature-arc would be simple and trivial but at the cost of performance. - Complexity increases when feature arc changes are optimally integrated (like "ip4_rewrite" changes in the patch) with no performance regression > > > I did not read much from Robin or Christophe who have been writing > > more node code than me. > > I would prefer their opinion before going forward. > > This series is indeed very dense. I like the concept of having > extensible sub trees in the graph but it feels like the implementation > is more complex than it should be. > > Lacking of another solution, we went for a naive approach in grout. > Basically, some nodes have undefined next nodes which are extended using > a dedicated API. Nitin> With an initial glance, it looks like "grout" is trying to solve a use-case where a child is being added to the parent's undefined next node. This is trying to create a runtime parent-child relationship On the other hand, feature arc not just create parent-child relationships but also sibling-sibling relationships as well. Also enabling sub-graph per interface is critical functionality in feature arc that adds complexity Let's assume a use-case in ingress direction, at the IPv4 layer, where IPv4-input is the *originating node* and - On interface X, IPsec-policy, IP4-classify() and IPv4-lookup sub-graphs are enabled in a sequential order - On interface Y, IP4-classify() and IPv4-lookup sub-graphs are enabled. in a sequential order. i.e. IPsec-policy is *disabled* on interface Y In fast path, following processing should happen for "mbuf0" which is received on interface "X" - "ipv4-input" sends mbuf0 to the first enabled sub-graph node for interface X, "IPsec-policy" - In "IPsec-policy" node processing, if policy action results in "bypass" action for mbuf0, it must then be sent to next enabled sub-graph i.e. "IPv4-classify" (from "IPsec-policy" node) - In "IPv4-classify" node processing, if classify fails for mbuf0 then it should finally be sent to "IPv4-lookup" node (from "IPv4-classify" node) whereas for "mbuf1" received on interface Y following fast path processing must happen - "Ipv4-input" sends mbuf1 to the first enabled sub-graph node for interface Y, "IPv4-classify" - If "IPv4-classify" fails for mbuf1, then it should finally be sent to IPv4-lookup node To behave differently for interface X and interface Y as above - First of all, IPsec-policy/IPv4-classify/IPv4-lookup must be connected to "ipv4-input" node (Parent-Child relationship) - Also, IPsec-policy/IPv4-classify/IPv4-lookup must also be connected with each other (Sibling-Sibling relationship) - Fast path APIs provide *rte_edges_t* to send mbuf from one node to another node 1. Based on interface (either Interface X or Interface Y) 2. Based on which node, fast path APIs are called. Next enabled feature/sub-graph can only be determined from previous enabled feature/sub-graph in fast path Not sure if grout handles above use-cases in the same manner. AFAIR , for any control plane change grout re-creates "graph" objects which may not be required with feature arc. > > https://github.com/DPDK/grout/blob/v0.2/modules/infra/datapath/eth_input.= c#L23-L31 > > This API can be used by other nodes to attach themselves to these > extensible nodes: > > https://github.com/DPDK/grout/blob/v0.2/modules/ip/datapath/arp_input.c#L= 143 > https://github.com/DPDK/grout/blob/v0.2/modules/ip/datapath/ip_input.c#L1= 24 > https://github.com/DPDK/grout/blob/v0.2/modules/ip6/datapath/ip6_input.c#= L122 > > After which, the extensible nodes can steer the packets towards the > correct downstream edge based on the dedicated classifier field: > > https://github.com/DPDK/grout/blob/v0.2/modules/infra/datapath/eth_input.= c#L79 > > Obviously, this does not natively support a per-interface sub tree > traversal, but it can be done in the originating node based on packet > private context data. Nitin> Expressing per interface sub-tree traversal is the key functionality of feature arc. > > This raises a more important question: how can we standardize the way > private application data is passed from node to node? And how could we > enforce this declaratively in the node register API? Nitin> What you are suggesting here (node to node application data exchange) can be done in rte_node_register API but, IMO, this is not related to feature arc. Feature arc is not just between parent and child nodes but also between siblings (as explained above) > > Do you think we could find some middle ground that would not require > such extensive changes? Nitin> If you are pointing to ipv4-rewrite changes, I had an internal review comment of adding those changes without any *performance regression*. A sub-optimal code would be much simpler but at the cost of performance. > > Cheers, > Robin >