David S. Miller wrote:
> When you guys decide on a final patch let me know, the semantic
> parts of Mitchell's changes look perfectly fine to me.
How about something like the following. It expands the comment but turns
that bit of code into an inline function so we onlt have to explain it
once.
Untested, but compiles fine.
-Mitch
--- linux-2.6.0-test5-VIRGIN/net/packet/af_packet.c 2003-09-08
12:39:53.000000000 -0700
+++ linux-2.6.0-test5mnb1/net/packet/af_packet.c 2003-09-15
10:56:26.313218168 -0700
@@ -381,6 +381,23 @@
}
#endif
+static inline unsigned run_filter(struct sk_buff *skb, struct sock *sk,
unsigned res)
+{
+ struct sk_filter *filter;
+
+ bh_lock_sock(sk);
+ filter = sk->sk_filter;
+ /*
+ * Our caller already checked that filter != NULL but we need to
+ * verify that under bh_lock_sock() to be safe
+ */
+ if (likely(filter != NULL))
+ res = sk_run_filter(skb, filter->insns, filter->len);
+ bh_unlock_sock(sk);
+
+ return res;
+}
+
/*
This function makes lazy skb cloning in hope that most of packets
are discarded by BPF.
@@ -429,15 +446,7 @@
snaplen = skb->len;
if (sk->sk_filter) {
- unsigned res = snaplen;
- struct sk_filter *filter;
-
- bh_lock_sock(sk);
- if ((filter = sk->sk_filter) != NULL)
- res = sk_run_filter(skb, sk->sk_filter->insns,
- sk->sk_filter->len);
- bh_unlock_sock(sk);
-
+ unsigned res = run_filter(skb, sk, snaplen);
if (res == 0)
goto drop_n_restore;
if (snaplen > res)
@@ -533,15 +542,7 @@
snaplen = skb->len;
if (sk->sk_filter) {
- unsigned res = snaplen;
- struct sk_filter *filter;
-
- bh_lock_sock(sk);
- if ((filter = sk->sk_filter) != NULL)
- res = sk_run_filter(skb, sk->sk_filter->insns,
- sk->sk_filter->len);
- bh_unlock_sock(sk);
-
+ unsigned res = run_filter(skb, sk, snaplen);
if (res == 0)
goto drop_n_restore;
if (snaplen > res)
|