| Message ID | 1505977929-13284-3-git-send-email-dlu998@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2,1/5] conntrack: Fix clang static analysis reports. | expand |
>Create a separate function from existing code, so the code can be reused in a >subsequent patch; no change in functionality. > >Signed-off-by: Darrell Ball <dlu998@gmail.com> >--- > lib/conntrack.c | 42 +++++++++++++++++++++++++++++------------- > 1 file changed, 29 insertions(+), 13 deletions(-) > >diff --git a/lib/conntrack.c b/lib/conntrack.c index c94bc27..2eca38d 100644 >--- a/lib/conntrack.c >+++ b/lib/conntrack.c >@@ -96,6 +96,11 @@ nat_conn_keys_lookup(struct hmap *nat_conn_keys, > const struct conn_key *key, > uint32_t basis); > >+static bool >+nat_conn_keys_insert(struct hmap *nat_conn_keys, >+ const struct conn *nat_conn, >+ uint32_t hash_basis); >+ This patch is refactoring the code with no change in functionality. Small nit (not necessarily needed) change variable name from 'hash_basis' to 'basis' to keep it consistent with other APIs in this file. LGTM Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
On 9/21/17, 3:19 AM, "ovs-dev-bounces@openvswitch.org on behalf of Bodireddy, Bhanuprakash" <ovs-dev-bounces@openvswitch.org on behalf of bhanuprakash.bodireddy@intel.com> wrote: >Create a separate function from existing code, so the code can be reused in a >subsequent patch; no change in functionality. > >Signed-off-by: Darrell Ball <dlu998@gmail.com> >--- > lib/conntrack.c | 42 +++++++++++++++++++++++++++++------------- > 1 file changed, 29 insertions(+), 13 deletions(-) > >diff --git a/lib/conntrack.c b/lib/conntrack.c index c94bc27..2eca38d 100644 >--- a/lib/conntrack.c >+++ b/lib/conntrack.c >@@ -96,6 +96,11 @@ nat_conn_keys_lookup(struct hmap *nat_conn_keys, > const struct conn_key *key, > uint32_t basis); > >+static bool >+nat_conn_keys_insert(struct hmap *nat_conn_keys, >+ const struct conn *nat_conn, >+ uint32_t hash_basis); >+ This patch is refactoring the code with no change in functionality. Small nit (not necessarily needed) change variable name from 'hash_basis' to 'basis' to keep it consistent with other APIs in this file. Good point, thanks. Darrell LGTM Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Qbh0dHzMRnUSaKLWNeJNOQKjnWEt2YnMTbIIynHei7k&s=ai_sMFBzkBffvyyZ4VVkcHiGm-OkBlXJOFFe_RINeQc&e=
diff --git a/lib/conntrack.c b/lib/conntrack.c index c94bc27..2eca38d 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -96,6 +96,11 @@ nat_conn_keys_lookup(struct hmap *nat_conn_keys, const struct conn_key *key, uint32_t basis); +static bool +nat_conn_keys_insert(struct hmap *nat_conn_keys, + const struct conn *nat_conn, + uint32_t hash_basis); + static void nat_conn_keys_remove(struct hmap *nat_conn_keys, const struct conn_key *key, @@ -2065,19 +2070,10 @@ nat_select_range_tuple(struct conntrack *ct, const struct conn *conn, nat_conn->rev_key.src.port = htons(port); } - struct nat_conn_key_node *nat_conn_key_node = - nat_conn_keys_lookup(&ct->nat_conn_keys, &nat_conn->rev_key, - ct->hash_basis); - - if (!nat_conn_key_node) { - struct nat_conn_key_node *nat_conn_key = - xzalloc(sizeof *nat_conn_key); - nat_conn_key->key = nat_conn->rev_key; - nat_conn_key->value = nat_conn->key; - uint32_t nat_conn_key_hash = conn_key_hash(&nat_conn_key->key, - ct->hash_basis); - hmap_insert(&ct->nat_conn_keys, &nat_conn_key->node, - nat_conn_key_hash); + bool new_insert = nat_conn_keys_insert(&ct->nat_conn_keys, nat_conn, + ct->hash_basis); + + if (new_insert) { return true; } else if (!all_ports_tried) { if (min_port == max_port) { @@ -2137,6 +2133,26 @@ nat_conn_keys_lookup(struct hmap *nat_conn_keys, return NULL; } +/* This function must be called with the ct->resources lock taken. */ +static bool +nat_conn_keys_insert(struct hmap *nat_conn_keys, const struct conn *nat_conn, + uint32_t hash_basis) +{ + struct nat_conn_key_node *nat_conn_key_node = + nat_conn_keys_lookup(nat_conn_keys, &nat_conn->rev_key, hash_basis); + + if (!nat_conn_key_node) { + struct nat_conn_key_node *nat_conn_key = xzalloc(sizeof *nat_conn_key); + nat_conn_key->key = nat_conn->rev_key; + nat_conn_key->value = nat_conn->key; + uint32_t nat_conn_key_hash = conn_key_hash(&nat_conn_key->key, + hash_basis); + hmap_insert(nat_conn_keys, &nat_conn_key->node, nat_conn_key_hash); + return true; + } + return false; +} + /* This function must be called with the ct->resources write lock taken. */ static void nat_conn_keys_remove(struct hmap *nat_conn_keys,
Create a separate function from existing code, so the code can be reused in a subsequent patch; no change in functionality. Signed-off-by: Darrell Ball <dlu998@gmail.com> --- lib/conntrack.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)