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 B40E8A0350; Tue, 30 Jun 2020 11:52:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 647911BEBF; Tue, 30 Jun 2020 11:52:40 +0200 (CEST) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id 629F61BEAA for ; Tue, 30 Jun 2020 11:52:38 +0200 (CEST) Received: by mail-io1-f65.google.com with SMTP id e64so15380748iof.12 for ; Tue, 30 Jun 2020 02:52:38 -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=Ck9nZpKBYWkyhPfQ5MpPippiGKwVrLIwhs1XHFonH9k=; b=LwK0GpJ+0Noxvor+4PhHbxpEG2b/Kkd88UcbFBocinaqwV5Z5MTVjiup4K9++Rqo8i jDU3DWORQEHEnlqiRH0orDAuEEbQt/9DUH45KDkXWEYPFL7tm1LYOkkLE1voyt+BmRMP nwO2E8k7hPxGILWtac2gDz3LHX691ZwRoJfW5+ZZ0UewQkUfbEheDyNeJKSocfqp6ovO ++3xBrB52Wq5sjfGfEpko46/a0O6DL6x1xs7YZV4lph9CwvGrCPQhzB4g7BFQrcN40BG NtLBzIsKTNAhWv+B866p3JJlCsknJFEUYXF0lC5L+dfkB/EchC1/0YHlJOPrl6saPyaK HNxA== 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=Ck9nZpKBYWkyhPfQ5MpPippiGKwVrLIwhs1XHFonH9k=; b=J6ctnNbOoLgP8qfvE/XLi59x6D09qY3tzLU5/uNKpDHg5aVUcOOiMxWDV/yhqnHS00 xLp4D2goNUcCImpzatchVIS/6mhDN3bmskAqYfDa61gU6IvRbYY2brQ44linkj32J2Z/ nJWCEvKzq8DkXdGM1KOXPdDMW7Agl1VJ+iYznZo9hI+lhc2YLc/tREoHoqe4NtdCqOU+ MAfih+7GFRo8xM2t3EPf3I4ZDMfE5uepOzNycsapsOusu1nwvQnzsxSfjTq+7ycZzAIA zGXRx520QfXDGLCYlIlwhyZESPz2ekwlIx2ZIOAhTAPNTpCBCRUpu5jinmdS0oJhdyvG t+gA== X-Gm-Message-State: AOAM5333jJebPoh1mAQR2ahRikFBtwk8FBFJZmYDUWpuiZb2fvJyIKM2 IyuuCnSyH4REV7zBJ4Bt3cPuaC3HAtRk4xIZCXE= X-Google-Smtp-Source: ABdhPJwc27Px1oawBZ2a3Ay11GASl75qR7uzpL8gQozhVO8MqNbJUj4WNbc2/etMsg6/j0+ibelaS3o+J+k3QginMKE= X-Received: by 2002:a02:2b24:: with SMTP id h36mr22530873jaa.104.1593510757495; Tue, 30 Jun 2020 02:52:37 -0700 (PDT) MIME-Version: 1.0 References: <20200620133231.12355-1-andrey.vesnovaty@gmail.com> In-Reply-To: From: Jerin Jacob Date: Tue, 30 Jun 2020 15:22:21 +0530 Message-ID: To: Andrey Vesnovaty Cc: Thomas Monjalon , dpdk-dev Content-Type: text/plain; charset="UTF-8" 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 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. > 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.