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 4FCF0A0350; Wed, 1 Jul 2020 12:34:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 299C41C2A7; Wed, 1 Jul 2020 12:34:58 +0200 (CEST) Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id CA61B1C21E for ; Wed, 1 Jul 2020 12:34:56 +0200 (CEST) Received: by mail-io1-f68.google.com with SMTP id v8so24335739iox.2 for ; Wed, 01 Jul 2020 03:34:56 -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:content-transfer-encoding; bh=GBuUJ22DPPGKKH9aRpuiDwRzMhkIOdLAOC6D+dOGLUg=; b=BUtf6Dir3szYzcgXFqis9mcJkyGwLEZO9iKmTgmiGVLa3GTClefb8tG1fMGjC3tMey Gae5F43E0snthz4AyJMuL8+fVQ57IKQ4EnHdCMt7tkCsfXZmMCkAyq1EaKpdfTtJJKvR j6evoF8Li+oq1cTSY9r54OqBV+DuTxoZCNC98i/v2UexXBbYWc8VrSpFdjB5iXw91JHu JHSXZ+qqp1q0RF+6b9RIW9ankHNpYNr1HDeLKWYXjulVe4xq7wLw7obDTLsgFp8+J6M9 fBJhktUB4iLBLouJ1ixg/AZSX6DLUUdQtWwqCzlTTsU3JW3uvpleeh1TAQ6nWLK1Y9+Y sU6w== 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:content-transfer-encoding; bh=GBuUJ22DPPGKKH9aRpuiDwRzMhkIOdLAOC6D+dOGLUg=; b=rPf9Mge8sHF6WECffhsJGZpOwa4ELLrp7f2Ybeajwzf5xE3LO/jijyVF5yjsEaVeQ+ d7ObIaeYgUf6A7vJUPljodRZMeoXYKEExCGjVAHS/Qr4UOa6YSMlWvl8Uwh08rEn2Bd9 WCrHyYarqPEcspZUa73DjSnr5S43+6xV60O0jNxyD3I6BM6mo99KH609LCttst401zl1 A2bqt0yI7XICBpu56MDWYKMB61XLmC/dhw4YJ9MsyKIElPzNMbZDVESExHeB37D/WtnJ c/YSbslmgI1SytFTA3HPReEtZFqxIBVmxesYAq+K0z5BGG/wlkHpOsRczt2BgQfjEGSF 4eRA== X-Gm-Message-State: AOAM530QAT8LoSOLJLovkd75cQjTI0wCdcBd0tvMMiMnKncbhePGZHwg xGiHZ0rK9TIWQsVywLil3Wr5zAymRChz900qKFz+0WFrvBM= X-Google-Smtp-Source: ABdhPJyJHpdXih9slJpVe1Xgn9II06w9eGv26t7cYYGGaMdcsNAwd6s8VK2Z/AoPeZJKHwPF9XDq8DlfVwSK87Oj5Hc= X-Received: by 2002:a05:6602:2d95:: with SMTP id k21mr1577488iow.59.1593599696099; Wed, 01 Jul 2020 03:34:56 -0700 (PDT) MIME-Version: 1.0 References: <20200620133231.12355-1-andrey.vesnovaty@gmail.com> In-Reply-To: From: Jerin Jacob Date: Wed, 1 Jul 2020 16:04:40 +0530 Message-ID: To: Andrey Vesnovaty Cc: Thomas Monjalon , dpdk-dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Wed, Jul 1, 2020 at 2:54 PM Andrey Vesnovaty wrote: > > > On Tue, Jun 30, 2020 at 12:52 PM Jerin Jacob wrot= e: >> >> On Mon, Jun 29, 2020 at 3:52 PM Andrey Vesnovaty >> wrote: >> > >> > >> > >> > On Sun, Jun 28, 2020 at 4:42 PM Jerin Jacob wr= ote: >> >> >> >> 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 an= y 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 descrip= tion was >> >> >> > incomplete & unclear. I hope v2 will be better at least in term= s 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 hav= e >> >> >> public API based on that level of abstraction. >> >> >> But at the PMD driver level we need to choose the correct abstract= ion >> >> >> 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 a= ction: >> > Based on this conversation I'm assuming that generic implementation su= pposed 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 lo= cking of each flow_create/flow_destroy in addition to action_uodate callbac= k 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 native= ly. Yes. That's what my concern with implementing shared session if the PMD only supports only action update for the given rte_flow *. Another approach would be to introduce rte_flow_action_update() public API which can either take "const struct rte_flow_action []" OR shared context ID, to cater to both cases or something on similar lines. >> >> >> > 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 sessi= on"? >> >> >> >> Yes. >> > >> > Common 'shared session' management code: >> > - can be reduced to atomic usage counter >> > - maintaining list of flow using shared action expected to impact perf= ormance & 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 prot= ected 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 influen= ced any more by updates or removed shared action.