Re: bpf filter : support for vlan tag

On Fri, Oct 19, 2012 at 11:02 PM, Ani Sinha [off-list ref] wrote:

On Fri, Oct 19, 2012 at 1:53 PM, Daniel Borkmann
[off-list ref] wrote:

quoted

On Fri, Oct 19, 2012 at 8:32 PM, Ani Sinha [off-list ref] wrote:

quoted

how about this?

On Tue, Oct 16, 2012 at 4:28 AM, Eric Dumazet [off-list ref] wrote:

quoted

@@ -341,6 +342,12 @@ load_b:
                case BPF_S_ANC_CPU:
                        A = raw_smp_processor_id();
                        continue;
+               case BPF_S_ANC_VLAN_TAG:
+                       A = vlan_tx_tag_get(skb);
+                       continue;
+               case BPF_S_ANC_VLAN_TAG_PRESENT:
+                       A = !!vlan_tx_tag_present(skb);
+                       continue;
                case BPF_S_ANC_NLATTR: {
                        struct nlattr *nla;

+               case BPF_S_ANC_VLAN_TAG:
+                       if (!vlan_tx_tag_present(skb)) {
+                               return 0;
+                       }
+                       A = vlan_tx_tag_get(skb);
+                       continue;

I didn't look into the code, but I assume that if no vlan is present,
then vlan_tx_tag_get might return 0 anyway.

This might not be true all the time. So it's always safe to do this
check before returning the VLANID and throw some kind of error if the
vlan ID is not set.

But wasn't this the reason why Eric added BPF_S_ANC_VLAN_TAG_PRESENT ?
Or is your objection performance related?

 Also, your return is

quoted

simply wrong, since then after this instruction you leave the *whole*
BPF machine ignoring the rest of the filter program to process ...

I had done that because I can see in other parts of that state machine
that in error condition the code simply stops processing the packet. I
am not sure how else to handle the error case.

See comment above. In my opinion the other cases are more severe like
divide by zero and the like, but okay ..