From fc2f2fa7d3eb6406307a38123fe171805113915f Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 25 May 2019 16:04:03 +0200 Subject: [PATCH] bypass: allow bypass for packet without flow For capture method that have their own flow structure (not maintained by Suricata), it can make sense to bypass a packet even if there is no Flow in Suricata. For AF_PACKET it does not make sense as the eBPF map entry will be destroyed as soon as it will be checked by the flow bypass manager. Thus we shortcut the bypass function if ever no Flow is attached to the packet. This path also removes reference to Flow in the bypass functions for AF_PACKET. It was not necessary and we possibly could benefit of it if ever we change the bypass algorithm. --- src/decode.c | 31 +++++++++++--------- src/source-af-packet.c | 66 ++++++++++++++++++++++++------------------ 2 files changed, 56 insertions(+), 41 deletions(-) diff --git a/src/decode.c b/src/decode.c index 77c4199044..ac6cf2a62d 100644 --- a/src/decode.c +++ b/src/decode.c @@ -402,22 +402,27 @@ void PacketBypassCallback(Packet *p) { /* Don't try to bypass if flow is already out or * if we have failed to do it once */ - int state = SC_ATOMIC_GET(p->flow->flow_state); - if ((state == FLOW_STATE_LOCAL_BYPASSED) || - (state == FLOW_STATE_CAPTURE_BYPASSED)) { - return; - } - - FlowCounters *fc = SCCalloc(sizeof(FlowCounters), 1); - if (fc) { - FlowSetStorageById(p->flow, GetFlowBypassCounterID(), fc); - } else { - return; + if (p->flow) { + int state = SC_ATOMIC_GET(p->flow->flow_state); + if ((state == FLOW_STATE_LOCAL_BYPASSED) || + (state == FLOW_STATE_CAPTURE_BYPASSED)) { + return; + } + FlowCounters *fc = SCCalloc(sizeof(FlowCounters), 1); + if (fc) { + FlowSetStorageById(p->flow, GetFlowBypassCounterID(), fc); + } else { + return; + } } if (p->BypassPacketsFlow && p->BypassPacketsFlow(p)) { - FlowUpdateState(p->flow, FLOW_STATE_CAPTURE_BYPASSED); + if (p->flow) { + FlowUpdateState(p->flow, FLOW_STATE_CAPTURE_BYPASSED); + } } else { - FlowUpdateState(p->flow, FLOW_STATE_LOCAL_BYPASSED); + if (p->flow) { + FlowUpdateState(p->flow, FLOW_STATE_LOCAL_BYPASSED); + } } } diff --git a/src/source-af-packet.c b/src/source-af-packet.c index 94aa68bece..8f023ce484 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -2291,12 +2291,9 @@ TmEcode AFPSetBPFFilter(AFPThreadVars *ptv) * * \param mapfd file descriptor of the protocol bypass table * \param key data to use as key in the table - * \param pkts_cnt packet count for the half flow - * \param bytes_cnt bytes count for the half flow * \return 0 in case of error, 1 if success */ static int AFPInsertHalfFlow(int mapd, void *key, uint32_t hash, - uint64_t pkts_cnt, uint64_t bytes_cnt, unsigned int nr_cpus) { BPF_DECLARE_PERCPU(struct pair, value, nr_cpus); @@ -2356,6 +2353,12 @@ static int AFPBypassCallback(Packet *p) return 0; } + /* If we don't have a flow attached to packet the eBPF map entries + * will be destroyed at first flow bypass manager pass as we won't + * find any associated entry */ + if (p->flow == NULL) { + return 0; + } /* Bypassing tunneled packets is currently not supported * because we can't discard the inner packet only due to * primitive parsing in eBPF */ @@ -2376,16 +2379,16 @@ static int AFPBypassCallback(Packet *p) key.vlan_id[1] = p->vlan_id[1]; key.ip_proto = IPV4_GET_IPPROTO(p); - if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, p->flow->todstpktcnt, - p->flow->todstbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } key.src = htonl(GET_IPV4_DST_ADDR_U32(p)); key.dst = htonl(GET_IPV4_SRC_ADDR_U32(p)); key.port16[0] = GET_TCP_DST_PORT(p); key.port16[1] = GET_TCP_SRC_PORT(p); - if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, p->flow->tosrcpktcnt, - p->flow->tosrcbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } EBPFUpdateFlow(p->flow, p, NULL); @@ -2409,8 +2412,8 @@ static int AFPBypassCallback(Packet *p) key.vlan_id[0] = p->vlan_id[0]; key.vlan_id[1] = p->vlan_id[1]; key.ip_proto = IPV6_GET_NH(p); - if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, p->flow->todstpktcnt, - p->flow->todstbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } for (i = 0; i < 4; i++) { @@ -2419,11 +2422,12 @@ static int AFPBypassCallback(Packet *p) } key.port16[0] = GET_TCP_DST_PORT(p); key.port16[1] = GET_TCP_SRC_PORT(p); - if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, p->flow->tosrcpktcnt, - p->flow->tosrcbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } - EBPFUpdateFlow(p->flow, p, NULL); + if (p->flow) + EBPFUpdateFlow(p->flow, p, NULL); return 1; } #endif @@ -2450,6 +2454,12 @@ static int AFPXDPBypassCallback(Packet *p) return 0; } + /* If we don't have a flow attached to packet the eBPF map entries + * will be destroyed at first flow bypass manager pass as we won't + * find any associated entry */ + if (p->flow == NULL) { + return 0; + } /* Bypassing tunneled packets is currently not supported * because we can't discard the inner packet only due to * primitive parsing in eBPF */ @@ -2461,25 +2471,25 @@ static int AFPXDPBypassCallback(Packet *p) if (p->afp_v.v4_map_fd == -1) { return 0; } - key.src = p->flow->src.addr_data32[0]; - key.dst = p->flow->dst.addr_data32[0]; + key.src = p->src.addr_data32[0]; + key.dst = p->dst.addr_data32[0]; /* In the XDP filter we get port from parsing of packet and not from skb * (as in eBPF filter) so we need to pass from host to network order */ - key.port16[0] = htons(p->flow->sp); - key.port16[1] = htons(p->flow->dp); + key.port16[0] = htons(p->sp); + key.port16[1] = htons(p->dp); key.vlan_id[0] = p->vlan_id[0]; key.vlan_id[1] = p->vlan_id[1]; key.ip_proto = IPV4_GET_IPPROTO(p); - if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, p->flow->todstpktcnt, - p->flow->todstbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } - key.src = p->flow->dst.addr_data32[0]; - key.dst = p->flow->src.addr_data32[0]; - key.port16[0] = htons(p->flow->dp); - key.port16[1] = htons(p->flow->sp); - if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, p->flow->tosrcpktcnt, - p->flow->tosrcbytecnt, p->afp_v.nr_cpus) == 0) { + key.src = p->dst.addr_data32[0]; + key.dst = p->src.addr_data32[0]; + key.port16[0] = htons(p->dp); + key.port16[1] = htons(p->sp); + if (AFPInsertHalfFlow(p->afp_v.v4_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } return 1; @@ -2502,8 +2512,8 @@ static int AFPXDPBypassCallback(Packet *p) key.vlan_id[0] = p->vlan_id[0]; key.vlan_id[1] = p->vlan_id[1]; key.ip_proto = IPV6_GET_NH(p); - if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, p->flow->todstpktcnt, - p->flow->todstbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } for (i = 0; i < 4; i++) { @@ -2512,8 +2522,8 @@ static int AFPXDPBypassCallback(Packet *p) } key.port16[0] = htons(GET_TCP_DST_PORT(p)); key.port16[1] = htons(GET_TCP_SRC_PORT(p)); - if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, p->flow->tosrcpktcnt, - p->flow->tosrcbytecnt, p->afp_v.nr_cpus) == 0) { + if (AFPInsertHalfFlow(p->afp_v.v6_map_fd, &key, p->flow_hash, + p->afp_v.nr_cpus) == 0) { return 0; } return 1; -- 2.47.2