On Wed, Feb 13, 2019 at 09:57:25PM -0800, Stanislav Fomichev wrote: > > > That 'stuck with __sk_buff' is what bothers me. > I might have use the wrong word here. I don't think there is another > option to be honest. Using __sk_buff makes flow dissector programs work > with fragmented packets;
good point. indeed real skb is essential. > > It's an indication that api wasn't thought through if first thing > > it needs is this fake skb hack. > > If bpf_flow.c is a realistic example of such flow dissector prog > > it means that real skb fields are accessed. > > In particular skb->vlan_proto, skb->protocol. > I do manually set skb->protocol to eth->h_proto in my proposal. This is later > correctly handled by bpf_flow.c: parse_eth_proto() is called on skb->protocol > and we correctly handle bpf_htons(ETH_P_8021Q) there. So existing > bpf_flow.c works as expected. ... > The goal of this patch series was to essentially make this skb/no-skb > context transparent to the bpf_flow.c (i.e. no changes from the user > flow programs). Adding another flow dissector for eth_get_headlen case > also seems as a no go. The problem with this thinking is assumption that bpf_flow.c is the only program. Since ctx of flow_dissector prog type is 'struct __sk_buff' all fields should be valid or the verifier has to reject access to fields that were not set. You cannot "manually set skb->protocol to eth->h_proto" in fake skb and ignore the rest.
