[dpdk-dev] [PATCH 06/12] eal: add channel for primary/secondary communication
Yuanhan Liu
yliu at fridaylinux.org
Fri Sep 29 03:24:37 CEST 2017
On Thu, Sep 28, 2017 at 01:50:20PM +0000, Tan, Jianfeng wrote:
> > > +int
> > > +rte_eal_primary_secondary_add_action(const char *action_name,
> > > + rte_eal_primary_secondary_t action)
> > > +{
> > > + struct action_entry *entry = malloc(sizeof(struct action_entry));
> > > +
> > > + if (entry == NULL)
> > > + return -ENOMEM;
> > > +
> > > + strncpy(entry->action_name, action_name,
> > MAX_ACTION_NAME_LEN);
> > > + entry->action = action;
> > > + TAILQ_INSERT_TAIL(&action_entry_list, entry, next);
> >
> > Since you intended to support "one primary process and multiple secondary
> > process", here we need a lock to protect the list.
>
> Only one thread of each process (either primary or secondary) does the register. So I wonder we don't have to add lock? Of course, no harm to add a lock.
Right, I was wrong. I was thinking secondary processes could start at the
same time, that we need a lock here. But as you said, the list is process
specific: each process has it's own copy. No locked is needed then.
> >
> > Another wonder is do we really need that, I mean 1:N model?
>
> I'm open to suggestions. IMO, not much extra code for 1:N model than 1:1 model. So not necessary to restrict that.
Fair enough. I was just wondering; I have no objection though.
> > > +/** Path of primary/secondary communication unix socket file. */
> > > +#define PRIMARY_SECONDARY_UNIX_PATH_FMT "%s/.%s_unix"
> > > +static inline const char *
> > > +eal_primary_secondary_unix_path(void)
> > > +{
> > > + static char buffer[PATH_MAX]; /* static so auto-zeroed */
> > > + const char *directory = default_config_dir;
> > > + const char *home_dir = getenv("HOME");
> >
> > It's not a good practice to generate such file at HOME dir. User would
> > be surprised to find it at HOME dir. In the worst case, user might delete
> > it.
>
> This way is the legacy way in DPDK, for example the config path. So I think we should fix that in another patch.
Yes, I think so.
--yliu
>
> >
> > The more common way is to put it to tmp dir, like "/tmp".
>
> Thanks,
> Jianfeng
More information about the dev
mailing list