[ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

[Ilya Maximets]

On 10.09.2019 14:11, Eelco Chaudron wrote:
> 
> 
> On 5 Sep 2019, at 14:40, Ilya Maximets wrote:
> 
>> Hi Eelco,
> 
> <SNIP>
>>> , 2 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index bc20d6843..a23150387 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -362,6 +362,7 @@ struct netdev_dpdk {
>>>          bool attached;
>>>          /* If true, rte_eth_dev_start() was successfully called */
>>>          bool started;
>>> +        bool reset_needed;
>>
>> This will produce a hole in the structure as members will no longer
>> fit into a single cacheline.  And cacheline markers will be misaligned.
>>
>> See details for similar issue here:
>> https://protect2.fireeye.com/url?k=66886626-3bef2200-6689ed69-0cc47a31384a-d13032efc289ec6c&u=https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362349.html
>>
>> Possible solution is to turn existing flags into bit fields or stop
>> avoiding the problem and drop most of padding in the structure.
>>
>>
>>>          struct eth_addr hwaddr;
>>>          int mtu;
>>>          int socket_id;
> 
> I did a pahole check, as my counting is always off, and it look ok, even with the change:
> 
> struct netdev_dpdk {
>     union {
>         OVS_CACHE_LINE_MARKER cacheline0;        /*     0     1 */
>         struct {
>             dpdk_port_t port_id;             /*     0     2 */
>             _Bool      attached;             /*     2     1 */
>             _Bool      started;              /*     3     1 */
>             _Bool      reset_needed;         /*     4     1 */
> 
>             /* XXX 1 byte hole, try to pack */
> 
>             struct eth_addr hwaddr;          /*     6     6 */
>             int        mtu;                  /*    12     4 */

OK. I see. There was a 2 byte hole after 'hwaddr' and with your patch
it moved earlier moving the 'hwaddr' from byte 4 to byte 6.
Maybe you could add a /* 1 pad byte here. */ after the new flag?

Best regards, Ilya Maximets.