DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrzej Ostruszka <amo@semihalf.com>
To: David Marchand <david.marchand@redhat.com>,
	Andrzej Ostruszka <aostruszka@marvell.com>
Cc: dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 0/4] Introduce IF proxy library
Date: Mon, 30 Mar 2020 21:23:24 +0200
Message-ID: <c65111e7-e8e8-6705-9ef9-864c4f8c7715@semihalf.com> (raw)
In-Reply-To: <55a2b3c7-b2f5-e4dc-6267-d8a27a2a8eee@semihalf.com>

On 3/26/20 1:41 PM, Andrzej Ostruszka wrote:
> Thank you David for taking time to look at this.
> 
> On 3/25/20 9:08 AM, David Marchand wrote:
>> Hello Andrzej,
>>
>> On Tue, Mar 10, 2020 at 12:11 PM Andrzej Ostruszka
> [...]
>> I can see we end up exposing structures for registering callbacks.
> 
> Right.  I was thinking more in terms of user convenience so it seemed
> like a good choice to gather them in one struct and call 'register'
> once.  The fact that the same structure is used to keep them is an
> implementation choice and this can be decoupled.
> 
>> Did you consider some ways to avoid exposure of those? (thinking of
>> ABI maintenance for when this library will elect to non-experimental).
> 
> I will.  So far I used the union for the input since I like when things
> are well typed :) and there is no need for casting.  However I will
> spend some time on this and will get back to you soon (if you have
> already something in your head please share).  Right now I'm thinking
> about taking array of callbacks with each entry being ("event type",
> callback) pair, however need to figure out how to have minimum amount of
> type casting.

David, I thought about this a bit and here is my proposal.

Define "typeful" callback pointer (public):

union rte_ifpx_cb_ptr {
    int (*mac_change)(const struct mac_change *ev);
    int (*mtu_change)(const struct mtu_change *ev);
    ...
    int (*cfg_done)(void);
};

In implementation make sure its size is as expected:

_Static_assert(sizeof(union rte_ifpx_cb_ptr) == sizeof (int(*)(void*)),
               "Size of callback pointer has to be"
               "equal to size of function pointer");

Accept as input tagged callbacks (also public type):

struct rte_ifpx_callback {
    enum rte_ifpx_event_type type;
    union rte_ifpx_cb_ptr callback;
};

The user would be defining array of callbacks:

struct rte_ifpx_callback callbacks[] = {
    {RTE_IFPX_MAC_CHANGE, {.mac_change = mac_change}},
    {RTE_IFPX_MTU_CHANGE, {.mtu_change = mtu_change}},
    ...
    {RTE_IFPX_CFG_DONE,   {.cfg_done   = finished}},
};

and passing it to registration together with its length like:

int rte_ifpx_callbacks_register(int len,
                                const struct rte_ifpx_callback *cbs)
{
    for (int i = 0; i < len; ++i) {
        switch (cbs[i].type) {
            case RTE_IFPX_MAC_CHANGE:
                priv_cbs.mac_change = cbs[i].callback.mac_change;
                break;
    ...
}

This way we should be protected from ABI breakage when adding new event
types and how the callbacks are stored would not be visible to the user.

Let me know what do you think about it.

With regards
Andrzej Ostruszka

  reply	other threads:[~2020-03-30 19:23 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 16:41 [dpdk-dev] [PATCH " Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-03-31 12:36   ` Harman Kalra
2020-03-31 15:37     ` Andrzej Ostruszka [C]
2020-04-01  5:29   ` Varghese, Vipin
2020-04-01 20:08     ` Andrzej Ostruszka [C]
2020-04-08  3:04       ` Varghese, Vipin
2020-04-08 18:13         ` Andrzej Ostruszka [C]
2020-03-06 16:41 ` [dpdk-dev] [PATCH 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-03-06 16:41 ` [dpdk-dev] [PATCH 4/4] if_proxy: add example application Andrzej Ostruszka
2020-03-06 17:17 ` [dpdk-dev] [PATCH 0/4] Introduce IF proxy library Andrzej Ostruszka
2020-03-10 11:10 ` [dpdk-dev] [PATCH v2 " Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-07-02  0:34     ` Stephen Hemminger
2020-07-07 20:13       ` Andrzej Ostruszka [C]
2020-07-08 16:07         ` Morten Brørup
2020-07-09  8:43           ` Andrzej Ostruszka [C]
2020-07-22  0:40             ` Thomas Monjalon
2020-07-22  8:45               ` Jerin Jacob
2020-07-22  8:56                 ` Thomas Monjalon
2020-07-22  9:09                   ` Jerin Jacob
2020-07-22  9:27                     ` Thomas Monjalon
2020-07-22  9:54                       ` Jerin Jacob
2020-07-23 14:09                         ` [dpdk-dev] [EXT] " Andrzej Ostruszka [C]
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-03-10 11:10   ` [dpdk-dev] [PATCH v2 4/4] if_proxy: add example application Andrzej Ostruszka
2020-03-25  8:08   ` [dpdk-dev] [PATCH v2 0/4] Introduce IF proxy library David Marchand
2020-03-25 11:11     ` Morten Brørup
2020-03-26 17:42       ` Andrzej Ostruszka
2020-04-02 13:48         ` Andrzej Ostruszka [C]
2020-04-03 17:19           ` Thomas Monjalon
2020-04-03 19:09             ` Jerin Jacob
2020-04-03 21:18               ` Morten Brørup
2020-04-03 21:57                 ` Thomas Monjalon
2020-04-04 10:18                   ` Jerin Jacob
2020-04-10 10:41                     ` Morten Brørup
2020-04-04 18:30             ` Andrzej Ostruszka [C]
2020-04-04 19:58               ` Thomas Monjalon
2020-04-10 10:03                 ` Morten Brørup
2020-04-10 12:28                   ` Jerin Jacob
2020-03-26 12:41     ` Andrzej Ostruszka
2020-03-30 19:23       ` Andrzej Ostruszka [this message]
2020-04-03 21:42   ` Thomas Monjalon
2020-04-04 18:07     ` Andrzej Ostruszka [C]
2020-04-04 19:51       ` Thomas Monjalon
2020-04-16 16:11 ` [dpdk-dev] [PATCH " Stephen Hemminger
2020-04-16 16:49   ` Jerin Jacob
2020-04-16 17:04     ` Stephen Hemminger
2020-04-16 17:26       ` Andrzej Ostruszka [C]
2020-04-16 17:27       ` Jerin Jacob
2020-04-16 17:12     ` Andrzej Ostruszka [C]
2020-04-16 17:19       ` Stephen Hemminger
2020-05-04  8:53 ` [dpdk-dev] [PATCH v3 " Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-05-04  8:53   ` [dpdk-dev] [PATCH v3 4/4] if_proxy: add example application Andrzej Ostruszka
2020-06-22  9:21 ` [dpdk-dev] [PATCH v4 0/4] Introduce IF proxy library Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 1/4] lib: introduce IF Proxy library Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 2/4] if_proxy: add library documentation Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 3/4] if_proxy: add simple functionality test Andrzej Ostruszka
2020-06-22  9:21   ` [dpdk-dev] [PATCH v4 4/4] if_proxy: add example application Andrzej Ostruszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c65111e7-e8e8-6705-9ef9-864c4f8c7715@semihalf.com \
    --to=amo@semihalf.com \
    --cc=aostruszka@marvell.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git