From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A9E6EA0350; Wed, 1 Jul 2020 11:24:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 654791C2F8; Wed, 1 Jul 2020 11:24:41 +0200 (CEST) Received: from mail-lj1-f179.google.com (mail-lj1-f179.google.com [209.85.208.179]) by dpdk.org (Postfix) with ESMTP id 1D6C81C2E8 for ; Wed, 1 Jul 2020 11:24:40 +0200 (CEST) Received: by mail-lj1-f179.google.com with SMTP id t25so21502460lji.12 for ; Wed, 01 Jul 2020 02:24:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=nyJgJKmltdG/vF4DDqx0S1i0AmeJnrsSTomgFuW5GwQ=; b=QYRSOkyCroaYJ25o8f/eQlUDIB8E/XK9gk0pBICe4tilLIWiC6lNG0UpjHcN7tQBeI VbNM0t5cWh3syxamwwyekktHTaPJSxaGm6tEd9klq3291MhoJw/RpcO02z0DNSXC6h5I AcO0uqa/k2yRc0pzKoKb7g1JIWLPxEkcfGufBes/o+jLYoGnS67kd1ghLZrxNwMGSu1u I8tUmVWc4PIK/MECyNLk2dywxOCNEx6aSYCy6L1ygLgfZA33oxhhqNfPwzRoqeAApepw UCq5mxi5XNCeea89hOPJChSnp/+ZOAq17VRyyZE78iBEYXO08SGAPKTL2wJL06lqBuYA rAQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nyJgJKmltdG/vF4DDqx0S1i0AmeJnrsSTomgFuW5GwQ=; b=k8z13xyygG+bL0zsN+uE89D3wKbIEW5AsfqP0ngCBwb0h/AMfNTbOinUGD7b8ekY2k UKKxVkOVP1MW2/sFap/7YTf/AWv2QnhOEmtcvtI/RB3QX5ytHu4ooZAwmSvdCWpQ1c4c ZCGxwYD4UhJ3cVIgnCz0n5Aeudszrxnl6u039WyXV6M/dxsXZf6KBgc2GxR5DYJE2v+z RbkRq5TURFt/yP+pbty19s6MkTvD5SqovfCIlxz+gdUqf5MhTwMLTbz6RThHlw9JJmVj zVAfc2MSEN2ybvL6+Bl0Kil/JbK2CrzGc6rcUnhcUq58Pa6BcQZ7zP4OaEPptEd2q4Jm liZg== X-Gm-Message-State: AOAM533B8tio0LPIDylEtMVEm20CW5siZ3J31fiYrbVI9abmkusWcw/V JUzQcTt3FepdtVY+yeIHSU7mlqEkkP5ETlmkR2M= X-Google-Smtp-Source: ABdhPJyD/8Iu+7rYad+ZIWLnHie9VA8jNXR5enUpTUOfhJwWJ1GJa8BNPxT6a8BbfNYloMxnmSUXcAeV5Usd/tNA8Hg= X-Received: by 2002:a2e:910c:: with SMTP id m12mr13537243ljg.332.1593595479701; Wed, 01 Jul 2020 02:24:39 -0700 (PDT) MIME-Version: 1.0 References: <20200620133231.12355-1-andrey.vesnovaty@gmail.com> In-Reply-To: From: Andrey Vesnovaty Date: Wed, 1 Jul 2020 12:24:28 +0300 Message-ID: To: Jerin Jacob Cc: Thomas Monjalon , dpdk-dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC v2 0/1] add flow action context API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jun 30, 2020 at 12:52 PM Jerin Jacob wrote: > On Mon, Jun 29, 2020 at 3:52 PM Andrey Vesnovaty > wrote: > > > > > > > > On Sun, Jun 28, 2020 at 4:42 PM Jerin Jacob > wrote: > >> > >> On Sun, Jun 28, 2020 at 2:14 PM Andrey Vesnovaty > >> wrote: > >> > > >> > Hi > >> > > >> > On Fri, Jun 26, 2020 at 2:44 PM Jerin Jacob > wrote: > >> >> > >> >> On Sat, Jun 20, 2020 at 7:02 PM Andrey Vesnovaty > >> >> wrote: > >> >> > > >> >> > Hi, and thanks a lot for your RFC v1 comments. > >> >> > > >> >> > RFC v2 emphasize the intent for sharing the flow action: > >> >> > * The term 'action context' was unclear and replaced with > >> >> > 'shared action'. > >> >> > * RFC v2 subject became 'add flow shared action API'. > >> >> > * all proposed APIs renamed according the above. > >> >> > > >> >> > The new shared action is an independent entity decoupled from any > flow > >> >> > while any flow can reuse such an action. Please go over the RFC > >> >> > description, it was almost entirely rewritten. > >> >> > > >> >> > @Jerin Jacob: > >> >> > Thanks again for your comments, it made me admit that v1 > description was > >> >> > incomplete & unclear. I hope v2 will be better at least in terms > of > >> >> > clarity. > >> >> > >> >> The public API and its usage is very clear. Thanks for this RFC. > >> > > >> > > >> > My pleasure. > >> >> > >> >> > >> >> I think, RFC v2 still not addressing the concern raised in the > >> >> http://mails.dpdk.org/archives/dev/2020-June/169296.html. > >> >> > >> >> Since MLX hardware has an HW based shared object it is fine to have > >> >> public API based on that level of abstraction. > >> >> But at the PMD driver level we need to choose the correct abstraction > >> >> to support all PMD and support shared object scheme if possible. > >> >> > >> >> I purpose to introduce something below or similar > >> >> int (*action_update) > >> >> (struct rte_eth_dev *, > >> >> struct rte_flow *flow, > >> >> const struct rte_flow_action [], > >> >> struct rte_flow_error *); > >> > > >> > Where this callback suppose to belong (struct rte_flow_ops)? > >> > >> Yes. > >> > >> > How should it be implemented by PMD? > >> > >> See below, > >> > >> > Is it about shared action and if "yes" why there is 'flow' argument? > >> > >> flow holds the "pattern" and "action" data as PMD specific handle. > >> So PMD, implementation can just change that action if it gets the PMD > >> specific handle. > >> > >> > >> >> > >> >> > >> >> in addition to: shared_action_create, shared_action_destroy, > >> >> shared_action_update, shared_action_query > >> >> > >> >> Have generic implementation of above, if action_update callback is > not > >> >> NULL. > >> > > >> > "is not NULL" -> "is NULL"? > >> > >> Yes. When it is NULL. > > > > > > Jerin, few clarifications regarding generic implementation of shared > action: > > Based on this conversation I'm assuming that generic implementation > supposed to be something like: > > For each flow using some shared action: > > call ops-> action_update() > > If the assumption above correct: > > 1. taking into account that shared_action_update() is atomic, how can > this deal with partial success: some flows may fail validation - should it: > > 1.1.lock all flows > > 1.2.validate all flows > > 1.3.update all flows > > 1.4. unlock > > Yes. > This kind of locking in addition to shared session management requires locking of each flow_create/flow_destroy in addition to action_uodate callback implementation even if there are no shared actions at all. In other words it imposes an overhead on all PMDs that don't support shared action natively. > > > 2. action_update callback is PMD specific & if it's unsupported there is > no support for shared action any way > > Yes. > > > Please address the issues above > > > > >> > > >> >> > >> >> So that, it can work all PMDs and to > >> >> avoid the duplication of "complex" shared session management code. > >> > > >> > Do you mean shared action in use by multiple flows by "shared > session"? > >> > >> Yes. > > > > Common 'shared session' management code: > > - can be reduced to atomic usage counter > > - maintaining list of flow using shared action expected to impact > performance & not necessary for all PMD specific implementations > > Access to other shared resources hard to generalize because: > > - for some PMDs mutual exclusion is HW feature & no need to protect it > in SW > > - for others there may be multiple resources & access to each one > protected by different mechanism > > The general callback you can assume, it supports only action_update > based callback. > If PMD has mutual exclusion HW feature then it can override the > function pointers. > > > > > An observation related to action_update callback: > > If replaced (updated) action was shared then the flow won't be > influenced any more by updates or removed shared action. >