Hi Thomas, <snip>
> > > Subject: Re: [dpdk-dev] [PATCH v5 01/13] librte_ether: modify > > > internal callback function > > > > > > 2016-10-06 12:26, Bernard Iremonger: > > > > void > > > > _rte_eth_dev_callback_process(struct rte_eth_dev *dev, > > > > - enum rte_eth_event_type event) > > > > + enum rte_eth_event_type event, void *param) > > > > > > You need to squash the patches updating the calls to this function. > > > Otherwise, this patch does not compile. > > > > I will have to squash everything into one patch, separate patches will not > compile. > > No you can keep a separate patch for the VF event in ixgbe. I have 4 patches at present librte_ether net/ixgbe drivers/net app/test Would this be acceptable or do you just want everything squashed into librte_ether except for net/ixgbe? > > > [...] > > > > + if (param != NULL) > > > > + dev_cb.cb_arg = (void *) param; > > > > > > You are overriding the user parameter. > > > > Yes, we want to update the user parameter for the > > RTE_ETH_EVENT_VF_MBOX I have renamed param to cb_arg to make it clearer what is happening. > > > As it is only for a new event, it can be described in the register > > > API that the user param won't be returned for this event. > > > > I will add a description in the rte_eth_dev_callback_register function. > > > > > But a better design would be to add a new parameter to the callback. > > > However it will be an API breakage. > > > > I do not want to break the ABI at this point. > > Yes, but it can be considered for a later change. Yes, ok > > > > + RTE_ETH_EVENT_VF_MBOX, /**< PF mailbox processing callback */ > > > > > > Sorry I do not parse well this line. > > > The event name is VF_MBOX and the comment is about the callback > > > processing this event on PF side? > > > I would suggest this kind of comment: "message from VF received by PF" > > > > Ok. > > > > > > > > [...] > > > > * Pointer to struct rte_eth_dev. > > > > * @param event > > > > * Eth device interrupt event type. > > > > + * @param param > > > > + * Parameter to pass back to user application. > > > > + * Allows the user application to decide if a particular > > > > + function > > > > + * is permitted. > > > > > > In a more generic case, the parameter gives some details about the > event. > > Please consider a rewording here, thanks. I have reworded here, I hope it is clearer. Regards, Bernard.
