He Yi(heyi-linaro) replied on github web page:
platform/linux-generic/pktio/loopback.c
line 69
@@ -251,18 +251,22 @@ static int loop_init_global(void)
return 0;
}
-const pktio_if_ops_t loopback_pktio_ops = {
- .name = "loop",
- .print = NULL,
- .init_global = loop_init_global,
- .init_local = NULL,
- .term = NULL,
+static pktio_ops_module_t loopback_pktio_ops = {
+ .base = {
+ .name = "loopback",
+ .init_local = NULL,
Comment:
Yes, will change it back to the original.
> He Yi(heyi-linaro) wrote:
> Yes, will fix all these.
>> He Yi(heyi-linaro) wrote:
>> Yes, created https://github.com/Linaro/odp/issues/184
>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> Do not change interface names as part of this PR. Any name changes should
>>> be a separate PR where we can access application impact. In the case of
>>> DPDK devices this may be appropriate here, but not as part of this PR.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> Same "to do" comment.
>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> Same "to do" comment.
>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> Again you've changed the interface name here from `socket_mmap` to
>>>>>> `socket mmap`. Keep the names unchanged as part of this PR to avoid
>>>>>> application impact.
>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> Same "to do" comment.
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> Same "to do" comment.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> This doesn't seem to go anywhere near the 80-char limit so should be
>>>>>>>>> fine as a single line.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> Would be better to split it after `static int` if that's the
>>>>>>>>>> problem, or at a parameter boundary. Splitting after the open paren
>>>>>>>>>> looks odd. For example:
>>>>>>>>>>
>>>>>>>>>> ```
>>>>>>>>>> static int _pcapif_parse_devname(pktio_ops_pcap_data_t *pcap,
>>>>>>>>>> const char *devname)
>>>>>>>>>> ```
>>>>>>>>>> would be normal ODP style here.
>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>> Same "to do" comment here.
>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>> As noted, "to dos" should be tracked as issues. These need not be
>>>>>>>>>>>> separate issues, as the main purpose is simply to remind us that
>>>>>>>>>>>> pktio still has work to do.
>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>> Why are we changing this pktio name? This will cause existing
>>>>>>>>>>>>> applications that use the "loop" interface to need to change,
>>>>>>>>>>>>> which seems unnecessary. I'd keep this PR strictly to
>>>>>>>>>>>>> modularization without any application impact. If we want to
>>>>>>>>>>>>> change the pktio name that should be a separate PR.
>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>> Please open an issue so that we don't forget about this "to do".
>>>>>>>>>>>>>> We decided that this is a good use for GitHub issues.
>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>> Run out of 80 characters?
>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>> Run out of 80 characters?
>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>> Yes, I'm thinking how to achieve this. Can it be later patch
>>>>>>>>>>>>>>>>> improvements.
>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>> This is temporary variables to make sure these modules are
>>>>>>>>>>>>>>>>>> linked into ODP library.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Because the Makefiles have not been re-organized to build
>>>>>>>>>>>>>>>>>> modules each as real independent module (.a or .so) and
>>>>>>>>>>>>>>>>>> either link them with --whole-archive or --no-as-needed
>>>>>>>>>>>>>>>>>> flags, this is necessary for current Makefiles.
>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>> Yes, here we only use the ODP_SUBSYSTEM_API() macro to
>>>>>>>>>>>>>>>>>>> typedef prototype function pointers and leave the prototype
>>>>>>>>>>>>>>>>>>> declarations no defines.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I suggest we accept this as is and consider how to handle
>>>>>>>>>>>>>>>>>>> this kind of tasks more properly in terms of autogen. as
>>>>>>>>>>>>>>>>>>> described in
>>>>>>>>>>>>>>>>>>> https://github.com/heyi-linaro/pure-interface/issues/6
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>> declaration fins in one line (there are others in this
>>>>>>>>>>>>>>>>>>>> file too)
>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>> indentation style
>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>> Can we find a solution so these structures are kept as
>>>>>>>>>>>>>>>>>>>>>> const? If I get this correctly, apart from the linked
>>>>>>>>>>>>>>>>>>>>>> list embedded in the "base class", there is nothing that
>>>>>>>>>>>>>>>>>>>>>> should be mutable here, right?
>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>> how and where are these `enable_link_*` variables used?
>>>>>>>>>>>>>>>>>>>>>>> How will this work with dynamically loadable modules?
>>>>>>>>>>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> Isn't this going to declare an external function
>>>>>>>>>>>>>>>>>>>>>>>> prototype called odp_pktio_ops_open(...)? However the
>>>>>>>>>>>>>>>>>>>>>>>> implementation won't exist, right?
>>>>>>>>>>>>>>>>>>>>>>>> I understand we need to define the function types
>>>>>>>>>>>>>>>>>>>>>>>> (odp_pktio_ops_open_t) for use in the structure below,
>>>>>>>>>>>>>>>>>>>>>>>> but do we need to declare all those function
>>>>>>>>>>>>>>>>>>>>>>>> prototypes in this header file?
>>>>>>>>>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>>>>>>>>>> We have the working solution currently in linux-dpdk.
>>>>>>>>>>>>>>>>>>>>>>>>> Question is, why we still want to have it in
>>>>>>>>>>>>>>>>>>>>>>>>> linux-generic ?
>>>>>>>>>>>>>>>>>>>>>>>>>> He Yi(heyi-linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> Yes, Krishna, as I replied in another email, I
>>>>>>>>>>>>>>>>>>>>>>>>>> suggest an approach that add workable solutions
>>>>>>>>>>>>>>>>>>>>>>>>>> first and after then remove obsolete code.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> With PR#140 it looks likes that we removed
>>>>>>>>>>>>>>>>>>>>>>>>>> linux-generic dpdk pktio and lost dpdk pktio at all,
>>>>>>>>>>>>>>>>>>>>>>>>>> now wait for a new design which seems still in
>>>>>>>>>>>>>>>>>>>>>>>>>> discussion.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> I'll comment these in your PR as a review suggestion.
>>>>>>>>>>>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>>>>>>>>>>>> I feel we don't need PKTIO_DPDK anylonger as we now
>>>>>>>>>>>>>>>>>>>>>>>>>>> already have it from linux-dpdk. I have sent a PR
>>>>>>>>>>>>>>>>>>>>>>>>>>> [https://github.com/Linaro/odp/pull/140](url) with
>>>>>>>>>>>>>>>>>>>>>>>>>>> necessary updates.
https://github.com/Linaro/odp/pull/139#discussion_r139859186
updated_at 2017-09-20 01:49:35
