Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/pktio/socket.c
line 170
@@ -873,9 +881,20 @@ const pktio_if_ops_t sock_mmsg_pktio_ops = {
.mac_get = sock_mac_addr_get,
.link_status = sock_link_status,
.capability = sock_capability,
- .pktin_ts_res = NULL,
- .pktin_ts_from_ns = NULL,
.config = NULL,
.input_queues_config = NULL,
.output_queues_config = NULL,
+ .print = NULL,
};
+
+ODP_MODULE_CONSTRUCTOR(socket_pktio_ops)
+{
+ odp_module_constructor(&socket_pktio_ops);
+
+ odp_subsystem_register_module(pktio_ops, &socket_pktio_ops);
+}
+
+/* Temporary variable to enable link this module,
+ * will remove in Makefile scheme changes.
+ */
+int enable_link_socket_pktio_ops = 0;
Comment:
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_r139839340
updated_at 2017-09-19 23:05:51
