On Wed, Aug 14, 2019 at 2:35 PM Darrell Ball <dlu...@gmail.com> wrote:
> On Mon, Aug 12, 2019 at 5:54 PM Yi-Hung Wei <yihung....@gmail.com> wrote:
>> --- a/lib/dpif-netlink.c
>> +++ b/lib/dpif-netlink.c
>> +static int
>> +dpif_netlink_ct_set_timeout_policy(struct dpif *dpif OVS_UNUSED,
>> + const struct ct_dpif_timeout_policy *tp)
>> +{
>> + struct nl_ct_timeout_policy nl_tp;
>> + struct ds nl_tp_name = DS_EMPTY_INITIALIZER;
>> + int i, err = 0;
>> +
>> + for (i = 0; i < ARRAY_SIZE(tp_protos); ++i) {
>
>
> for (int i = 0; i < ARRAY_SIZE(tp_protos); ++i) {
>
> there are several other cases as well
> I not going to mention all
Thanks for the review. I make all the similar code changes in
ct-dpif.c, dpif-netlink.c and netlink-conntrack.c.
>> +static int
>> +dpif_netlink_ct_timeout_policy_dump_next(struct dpif *dpif OVS_UNUSED,
>> + void *state,
>> + struct ct_dpif_timeout_policy *tp)
>
> I think it would be super helpful to add some comments to this function.
>
Sure, I added some comments to dpif_netlink_ct_timeout_policy_dump_next() in v4.
>> + tp_dump_node = get_dpif_netlink_tp_dump_node_by_tp_id(
>> + tp_id, &dump_state->tp_dump_map);
>> + if (!tp_dump_node) {
>> + tp_dump_node = xzalloc(sizeof *tp_dump_node);
>> + tp_dump_node->tp = xzalloc(sizeof *tp_dump_node->tp);
>> + tp_dump_node->tp->id = tp_id;
>> + hmap_insert(&dump_state->tp_dump_map, &tp_dump_node->hmap_node,
>> + hash_int(tp_id, 0));
>> + }
>> +
>> + update_dpif_netlink_tp_dump_node(&nl_tp, tp_dump_node);
>> + if (tp_dump_node->l3_l4_present == DPIF_NL_ALL_TP) {
>
>>
>> + hmap_remove(&dump_state->tp_dump_map, &tp_dump_node->hmap_node);
>> + *tp = *tp_dump_node->tp;
>> + free(tp_dump_node->tp);
>> + free(tp_dump_node);
>
> common block; write once
Sure, I move the common block to a new function in v4.
static void
get_and_cleanup_tp_dump_node(struct hmap *hamp,
struct dpif_netlink_tp_dump_node *tp_dump_node,
struct ct_dpif_timeout_policy *tp)
{
hmap_remove(hmap, &tp_dump_node->hmap_node);
*tp = *tp_dump_node->tp;
free(tp_dump_node->tp);
free(tp_dump_node);
}
Thanks,
-Yi-Hung
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
