[ovs-dev] [PATCH 2/7] dpif-netdev: retrieve flow directly from the flow mark

Yuanhan Liu yliu at fridaylinux.org
Wed Aug 30 02:27:37 UTC 2017


On Wed, Aug 30, 2017 at 01:55:28AM +0000, Darrell Ball wrote:
> 
> 
> On 8/29/17, 5:09 AM, "Yuanhan Liu" <yliu at fridaylinux.org> wrote:
> 
>     On Tue, Aug 29, 2017 at 07:14:48AM +0000, Darrell Ball wrote:
>     > Hi
>     > 
>     > There is a build issue after patch 2 is applied
>     > 
>     > f-netdev.Tpo -c ../lib/dpif-netdev.c -o lib/dpif-netdev.o
>     > ../lib/dpif-netdev.c: In function 'emc_processing':
>     > ../lib/dpif-netdev.c:4782:21: error: excess elements in struct initializer [-Werror]
>     >                      miniflow_values(&fake_mf),
>     >                      ^
>     > ../lib/dpif-netdev.c:4782:21: error: (near initialization for 'fake_mf') [-Werror]
>     > ../lib/dpif-netdev.c:4784:17: error: excess elements in struct initializer [-Werror]
>     >                  };
>     >                  ^
>     > ../lib/dpif-netdev.c:4784:17: error: (near initialization for 'fake_mf') [-Werror]
>     > cc1: all warnings being treated as errors
>     
>     Yes, I have noticed this warning. I haven't really looked at it though,
>     because there is a more important issue to fix: the tcp_flags counter
>     is ignored in this patchset. The possible solutions I can think of so
>     far are:
>     
>     - build a fake miniflow just with the tcp_flags field being checked.
>       However, it adds back the heavy overhead (checking each pkt) this
>       patchset meant to skip. I don't know how much overhead it may
>       introduce though. I could do some experiment. But I think the
>       functionality outweighs the performance.
>     
>     - just ignore it. I'm thinking this is obviously wrong?
> 
> We cannot ignore it

Yes, I know. I just want to know how bad it could be if we ignore it.

>     - query the tcp_flags counters from the hardware. I'm just aware of
>       that some NICs are able to offload it, nothing more.
>     
>     That being said, even though it could be offloaded by some hardwares,
>     it could not for some other hardwares. Meaning, the SW fallback is
>     still needed. I will measure the overhead in the next version.
> 
> 
> We would kinda need to fix the warning/error in some way.
> Did you intend struct mf_ctx instead of struct miniflow ?

Honestly, I don't know. I'm still quite rusty to it (and to OVS).
But no worry, I will figure it out.

	--yliu


More information about the dev mailing list