Commit c4c2dc54 authored by Jiri Pirko's avatar Jiri Pirko Committed by David S. Miller

mlxsw: spectrum_acl: Split entry struct into entry and ventry

Do the split of entry struct so the new entry struct is related to the
actual HW entry, whereas ventry struct is a SW abstration of that.
This split prepares possibility for ventry to hold 2 HW entries which
is needed for region ERP rehash flow.
Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarIdo Schimmel <idosch@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b2d6b4d2
...@@ -640,7 +640,7 @@ mlxsw_sp_acl_rule_create(struct mlxsw_sp *mlxsw_sp, ...@@ -640,7 +640,7 @@ mlxsw_sp_acl_rule_create(struct mlxsw_sp *mlxsw_sp,
int err; int err;
mlxsw_sp_acl_ruleset_ref_inc(ruleset); mlxsw_sp_acl_ruleset_ref_inc(ruleset);
rule = kzalloc(sizeof(*rule) + ops->rule_priv_size(mlxsw_sp), rule = kzalloc(sizeof(*rule) + ops->rule_priv_size,
GFP_KERNEL); GFP_KERNEL);
if (!rule) { if (!rule) {
err = -ENOMEM; err = -ENOMEM;
......
...@@ -190,11 +190,16 @@ struct mlxsw_sp_acl_tcam_vchunk { ...@@ -190,11 +190,16 @@ struct mlxsw_sp_acl_tcam_vchunk {
}; };
struct mlxsw_sp_acl_tcam_entry { struct mlxsw_sp_acl_tcam_entry {
struct mlxsw_sp_acl_tcam_vchunk *vchunk; struct mlxsw_sp_acl_tcam_ventry *ventry;
unsigned long priv[0]; unsigned long priv[0];
/* priv has to be always the last item */ /* priv has to be always the last item */
}; };
struct mlxsw_sp_acl_tcam_ventry {
struct mlxsw_sp_acl_tcam_entry *entry;
struct mlxsw_sp_acl_tcam_vchunk *vchunk;
};
static const struct rhashtable_params mlxsw_sp_acl_tcam_vchunk_ht_params = { static const struct rhashtable_params mlxsw_sp_acl_tcam_vchunk_ht_params = {
.key_len = sizeof(unsigned int), .key_len = sizeof(unsigned int),
.key_offset = offsetof(struct mlxsw_sp_acl_tcam_vchunk, priority), .key_offset = offsetof(struct mlxsw_sp_acl_tcam_vchunk, priority),
...@@ -828,65 +833,52 @@ mlxsw_sp_acl_tcam_vchunk_put(struct mlxsw_sp *mlxsw_sp, ...@@ -828,65 +833,52 @@ mlxsw_sp_acl_tcam_vchunk_put(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_acl_tcam_vchunk_destroy(mlxsw_sp, vchunk); mlxsw_sp_acl_tcam_vchunk_destroy(mlxsw_sp, vchunk);
} }
static size_t mlxsw_sp_acl_tcam_entry_priv_size(struct mlxsw_sp *mlxsw_sp) static struct mlxsw_sp_acl_tcam_entry *
{ mlxsw_sp_acl_tcam_entry_create(struct mlxsw_sp *mlxsw_sp,
const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops; struct mlxsw_sp_acl_tcam_ventry *ventry,
struct mlxsw_sp_acl_tcam_region *region,
return ops->entry_priv_size; struct mlxsw_sp_acl_tcam_chunk *chunk,
}
static int mlxsw_sp_acl_tcam_entry_add(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_group *group,
struct mlxsw_sp_acl_tcam_entry *entry,
struct mlxsw_sp_acl_rule_info *rulei) struct mlxsw_sp_acl_rule_info *rulei)
{ {
const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops; const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops;
struct mlxsw_sp_acl_tcam_vchunk *vchunk; struct mlxsw_sp_acl_tcam_entry *entry;
struct mlxsw_sp_acl_tcam_region *region;
struct mlxsw_sp_acl_tcam_chunk *chunk;
int err; int err;
vchunk = mlxsw_sp_acl_tcam_vchunk_get(mlxsw_sp, group, rulei->priority, entry = kzalloc(sizeof(*entry) + ops->entry_priv_size, GFP_KERNEL);
&rulei->values.elusage); if (!entry)
if (IS_ERR(vchunk)) return ERR_PTR(-ENOMEM);
return PTR_ERR(vchunk); entry->ventry = ventry;
chunk = vchunk->chunk;
region = vchunk->vregion->region;
err = ops->entry_add(mlxsw_sp, region->priv, chunk->priv, err = ops->entry_add(mlxsw_sp, region->priv, chunk->priv,
entry->priv, rulei); entry->priv, rulei);
if (err) if (err)
goto err_entry_add; goto err_entry_add;
entry->vchunk = vchunk;
return 0; return entry;
err_entry_add: err_entry_add:
mlxsw_sp_acl_tcam_vchunk_put(mlxsw_sp, vchunk); kfree(entry);
return err; return ERR_PTR(err);
} }
static void mlxsw_sp_acl_tcam_entry_del(struct mlxsw_sp *mlxsw_sp, static void mlxsw_sp_acl_tcam_entry_destroy(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_region *region,
struct mlxsw_sp_acl_tcam_chunk *chunk,
struct mlxsw_sp_acl_tcam_entry *entry) struct mlxsw_sp_acl_tcam_entry *entry)
{ {
const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops; const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops;
struct mlxsw_sp_acl_tcam_vchunk *vchunk = entry->vchunk;
struct mlxsw_sp_acl_tcam_chunk *chunk = vchunk->chunk;
struct mlxsw_sp_acl_tcam_region *region = vchunk->vregion->region;
ops->entry_del(mlxsw_sp, region->priv, chunk->priv, entry->priv); ops->entry_del(mlxsw_sp, region->priv, chunk->priv, entry->priv);
mlxsw_sp_acl_tcam_vchunk_put(mlxsw_sp, vchunk); kfree(entry);
} }
static int static int
mlxsw_sp_acl_tcam_entry_action_replace(struct mlxsw_sp *mlxsw_sp, mlxsw_sp_acl_tcam_entry_action_replace(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_region *region,
struct mlxsw_sp_acl_tcam_entry *entry, struct mlxsw_sp_acl_tcam_entry *entry,
struct mlxsw_sp_acl_rule_info *rulei) struct mlxsw_sp_acl_rule_info *rulei)
{ {
const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops; const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops;
struct mlxsw_sp_acl_tcam_vchunk *vchunk = entry->vchunk;
struct mlxsw_sp_acl_tcam_region *region = vchunk->vregion->region;
return ops->entry_action_replace(mlxsw_sp, region->priv, return ops->entry_action_replace(mlxsw_sp, region->priv,
entry->priv, rulei); entry->priv, rulei);
...@@ -894,17 +886,79 @@ mlxsw_sp_acl_tcam_entry_action_replace(struct mlxsw_sp *mlxsw_sp, ...@@ -894,17 +886,79 @@ mlxsw_sp_acl_tcam_entry_action_replace(struct mlxsw_sp *mlxsw_sp,
static int static int
mlxsw_sp_acl_tcam_entry_activity_get(struct mlxsw_sp *mlxsw_sp, mlxsw_sp_acl_tcam_entry_activity_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_region *region,
struct mlxsw_sp_acl_tcam_entry *entry, struct mlxsw_sp_acl_tcam_entry *entry,
bool *activity) bool *activity)
{ {
const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops; const struct mlxsw_sp_acl_tcam_ops *ops = mlxsw_sp->acl_tcam_ops;
struct mlxsw_sp_acl_tcam_vchunk *vchunk = entry->vchunk;
struct mlxsw_sp_acl_tcam_region *region = vchunk->vregion->region;
return ops->entry_activity_get(mlxsw_sp, region->priv, return ops->entry_activity_get(mlxsw_sp, region->priv,
entry->priv, activity); entry->priv, activity);
} }
static int mlxsw_sp_acl_tcam_ventry_add(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_group *group,
struct mlxsw_sp_acl_tcam_ventry *ventry,
struct mlxsw_sp_acl_rule_info *rulei)
{
struct mlxsw_sp_acl_tcam_vchunk *vchunk;
int err;
vchunk = mlxsw_sp_acl_tcam_vchunk_get(mlxsw_sp, group, rulei->priority,
&rulei->values.elusage);
if (IS_ERR(vchunk))
return PTR_ERR(vchunk);
ventry->vchunk = vchunk;
ventry->entry = mlxsw_sp_acl_tcam_entry_create(mlxsw_sp, ventry,
vchunk->vregion->region,
vchunk->chunk, rulei);
if (IS_ERR(ventry->entry)) {
err = PTR_ERR(ventry->entry);
goto err_entry_create;
}
return 0;
err_entry_create:
mlxsw_sp_acl_tcam_vchunk_put(mlxsw_sp, vchunk);
return err;
}
static void mlxsw_sp_acl_tcam_ventry_del(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_ventry *ventry)
{
struct mlxsw_sp_acl_tcam_vchunk *vchunk = ventry->vchunk;
mlxsw_sp_acl_tcam_entry_destroy(mlxsw_sp, vchunk->vregion->region,
vchunk->chunk, ventry->entry);
mlxsw_sp_acl_tcam_vchunk_put(mlxsw_sp, vchunk);
}
static int
mlxsw_sp_acl_tcam_ventry_action_replace(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_ventry *ventry,
struct mlxsw_sp_acl_rule_info *rulei)
{
struct mlxsw_sp_acl_tcam_vchunk *vchunk = ventry->vchunk;
return mlxsw_sp_acl_tcam_entry_action_replace(mlxsw_sp,
vchunk->vregion->region,
ventry->entry, rulei);
}
static int
mlxsw_sp_acl_tcam_ventry_activity_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_ventry *ventry,
bool *activity)
{
struct mlxsw_sp_acl_tcam_vchunk *vchunk = ventry->vchunk;
return mlxsw_sp_acl_tcam_entry_activity_get(mlxsw_sp,
vchunk->vregion->region,
ventry->entry, activity);
}
static const enum mlxsw_afk_element mlxsw_sp_acl_tcam_pattern_ipv4[] = { static const enum mlxsw_afk_element mlxsw_sp_acl_tcam_pattern_ipv4[] = {
MLXSW_AFK_ELEMENT_SRC_SYS_PORT, MLXSW_AFK_ELEMENT_SRC_SYS_PORT,
MLXSW_AFK_ELEMENT_DMAC_32_47, MLXSW_AFK_ELEMENT_DMAC_32_47,
...@@ -959,7 +1013,7 @@ struct mlxsw_sp_acl_tcam_flower_ruleset { ...@@ -959,7 +1013,7 @@ struct mlxsw_sp_acl_tcam_flower_ruleset {
}; };
struct mlxsw_sp_acl_tcam_flower_rule { struct mlxsw_sp_acl_tcam_flower_rule {
struct mlxsw_sp_acl_tcam_entry entry; struct mlxsw_sp_acl_tcam_ventry ventry;
}; };
static int static int
...@@ -1017,12 +1071,6 @@ mlxsw_sp_acl_tcam_flower_ruleset_group_id(void *ruleset_priv) ...@@ -1017,12 +1071,6 @@ mlxsw_sp_acl_tcam_flower_ruleset_group_id(void *ruleset_priv)
return mlxsw_sp_acl_tcam_group_id(&ruleset->group); return mlxsw_sp_acl_tcam_group_id(&ruleset->group);
} }
static size_t mlxsw_sp_acl_tcam_flower_rule_priv_size(struct mlxsw_sp *mlxsw_sp)
{
return sizeof(struct mlxsw_sp_acl_tcam_flower_rule) +
mlxsw_sp_acl_tcam_entry_priv_size(mlxsw_sp);
}
static int static int
mlxsw_sp_acl_tcam_flower_rule_add(struct mlxsw_sp *mlxsw_sp, mlxsw_sp_acl_tcam_flower_rule_add(struct mlxsw_sp *mlxsw_sp,
void *ruleset_priv, void *rule_priv, void *ruleset_priv, void *rule_priv,
...@@ -1031,8 +1079,8 @@ mlxsw_sp_acl_tcam_flower_rule_add(struct mlxsw_sp *mlxsw_sp, ...@@ -1031,8 +1079,8 @@ mlxsw_sp_acl_tcam_flower_rule_add(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam_flower_ruleset *ruleset = ruleset_priv; struct mlxsw_sp_acl_tcam_flower_ruleset *ruleset = ruleset_priv;
struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv;
return mlxsw_sp_acl_tcam_entry_add(mlxsw_sp, &ruleset->group, return mlxsw_sp_acl_tcam_ventry_add(mlxsw_sp, &ruleset->group,
&rule->entry, rulei); &rule->ventry, rulei);
} }
static void static void
...@@ -1040,7 +1088,7 @@ mlxsw_sp_acl_tcam_flower_rule_del(struct mlxsw_sp *mlxsw_sp, void *rule_priv) ...@@ -1040,7 +1088,7 @@ mlxsw_sp_acl_tcam_flower_rule_del(struct mlxsw_sp *mlxsw_sp, void *rule_priv)
{ {
struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv;
mlxsw_sp_acl_tcam_entry_del(mlxsw_sp, &rule->entry); mlxsw_sp_acl_tcam_ventry_del(mlxsw_sp, &rule->ventry);
} }
static int static int
...@@ -1057,7 +1105,7 @@ mlxsw_sp_acl_tcam_flower_rule_activity_get(struct mlxsw_sp *mlxsw_sp, ...@@ -1057,7 +1105,7 @@ mlxsw_sp_acl_tcam_flower_rule_activity_get(struct mlxsw_sp *mlxsw_sp,
{ {
struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_flower_rule *rule = rule_priv;
return mlxsw_sp_acl_tcam_entry_activity_get(mlxsw_sp, &rule->entry, return mlxsw_sp_acl_tcam_ventry_activity_get(mlxsw_sp, &rule->ventry,
activity); activity);
} }
...@@ -1068,7 +1116,7 @@ static const struct mlxsw_sp_acl_profile_ops mlxsw_sp_acl_tcam_flower_ops = { ...@@ -1068,7 +1116,7 @@ static const struct mlxsw_sp_acl_profile_ops mlxsw_sp_acl_tcam_flower_ops = {
.ruleset_bind = mlxsw_sp_acl_tcam_flower_ruleset_bind, .ruleset_bind = mlxsw_sp_acl_tcam_flower_ruleset_bind,
.ruleset_unbind = mlxsw_sp_acl_tcam_flower_ruleset_unbind, .ruleset_unbind = mlxsw_sp_acl_tcam_flower_ruleset_unbind,
.ruleset_group_id = mlxsw_sp_acl_tcam_flower_ruleset_group_id, .ruleset_group_id = mlxsw_sp_acl_tcam_flower_ruleset_group_id,
.rule_priv_size = mlxsw_sp_acl_tcam_flower_rule_priv_size, .rule_priv_size = sizeof(struct mlxsw_sp_acl_tcam_flower_rule),
.rule_add = mlxsw_sp_acl_tcam_flower_rule_add, .rule_add = mlxsw_sp_acl_tcam_flower_rule_add,
.rule_del = mlxsw_sp_acl_tcam_flower_rule_del, .rule_del = mlxsw_sp_acl_tcam_flower_rule_del,
.rule_action_replace = mlxsw_sp_acl_tcam_flower_rule_action_replace, .rule_action_replace = mlxsw_sp_acl_tcam_flower_rule_action_replace,
...@@ -1081,7 +1129,7 @@ struct mlxsw_sp_acl_tcam_mr_ruleset { ...@@ -1081,7 +1129,7 @@ struct mlxsw_sp_acl_tcam_mr_ruleset {
}; };
struct mlxsw_sp_acl_tcam_mr_rule { struct mlxsw_sp_acl_tcam_mr_rule {
struct mlxsw_sp_acl_tcam_entry entry; struct mlxsw_sp_acl_tcam_ventry ventry;
}; };
static int static int
...@@ -1155,12 +1203,6 @@ mlxsw_sp_acl_tcam_mr_ruleset_group_id(void *ruleset_priv) ...@@ -1155,12 +1203,6 @@ mlxsw_sp_acl_tcam_mr_ruleset_group_id(void *ruleset_priv)
return mlxsw_sp_acl_tcam_group_id(&ruleset->group); return mlxsw_sp_acl_tcam_group_id(&ruleset->group);
} }
static size_t mlxsw_sp_acl_tcam_mr_rule_priv_size(struct mlxsw_sp *mlxsw_sp)
{
return sizeof(struct mlxsw_sp_acl_tcam_mr_rule) +
mlxsw_sp_acl_tcam_entry_priv_size(mlxsw_sp);
}
static int static int
mlxsw_sp_acl_tcam_mr_rule_add(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv, mlxsw_sp_acl_tcam_mr_rule_add(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv,
void *rule_priv, void *rule_priv,
...@@ -1169,8 +1211,8 @@ mlxsw_sp_acl_tcam_mr_rule_add(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv, ...@@ -1169,8 +1211,8 @@ mlxsw_sp_acl_tcam_mr_rule_add(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv,
struct mlxsw_sp_acl_tcam_mr_ruleset *ruleset = ruleset_priv; struct mlxsw_sp_acl_tcam_mr_ruleset *ruleset = ruleset_priv;
struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv;
return mlxsw_sp_acl_tcam_entry_add(mlxsw_sp, &ruleset->group, return mlxsw_sp_acl_tcam_ventry_add(mlxsw_sp, &ruleset->group,
&rule->entry, rulei); &rule->ventry, rulei);
} }
static void static void
...@@ -1178,7 +1220,7 @@ mlxsw_sp_acl_tcam_mr_rule_del(struct mlxsw_sp *mlxsw_sp, void *rule_priv) ...@@ -1178,7 +1220,7 @@ mlxsw_sp_acl_tcam_mr_rule_del(struct mlxsw_sp *mlxsw_sp, void *rule_priv)
{ {
struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv;
mlxsw_sp_acl_tcam_entry_del(mlxsw_sp, &rule->entry); mlxsw_sp_acl_tcam_ventry_del(mlxsw_sp, &rule->ventry);
} }
static int static int
...@@ -1188,7 +1230,7 @@ mlxsw_sp_acl_tcam_mr_rule_action_replace(struct mlxsw_sp *mlxsw_sp, ...@@ -1188,7 +1230,7 @@ mlxsw_sp_acl_tcam_mr_rule_action_replace(struct mlxsw_sp *mlxsw_sp,
{ {
struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv;
return mlxsw_sp_acl_tcam_entry_action_replace(mlxsw_sp, &rule->entry, return mlxsw_sp_acl_tcam_ventry_action_replace(mlxsw_sp, &rule->ventry,
rulei); rulei);
} }
...@@ -1198,7 +1240,7 @@ mlxsw_sp_acl_tcam_mr_rule_activity_get(struct mlxsw_sp *mlxsw_sp, ...@@ -1198,7 +1240,7 @@ mlxsw_sp_acl_tcam_mr_rule_activity_get(struct mlxsw_sp *mlxsw_sp,
{ {
struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv; struct mlxsw_sp_acl_tcam_mr_rule *rule = rule_priv;
return mlxsw_sp_acl_tcam_entry_activity_get(mlxsw_sp, &rule->entry, return mlxsw_sp_acl_tcam_ventry_activity_get(mlxsw_sp, &rule->ventry,
activity); activity);
} }
...@@ -1209,7 +1251,7 @@ static const struct mlxsw_sp_acl_profile_ops mlxsw_sp_acl_tcam_mr_ops = { ...@@ -1209,7 +1251,7 @@ static const struct mlxsw_sp_acl_profile_ops mlxsw_sp_acl_tcam_mr_ops = {
.ruleset_bind = mlxsw_sp_acl_tcam_mr_ruleset_bind, .ruleset_bind = mlxsw_sp_acl_tcam_mr_ruleset_bind,
.ruleset_unbind = mlxsw_sp_acl_tcam_mr_ruleset_unbind, .ruleset_unbind = mlxsw_sp_acl_tcam_mr_ruleset_unbind,
.ruleset_group_id = mlxsw_sp_acl_tcam_mr_ruleset_group_id, .ruleset_group_id = mlxsw_sp_acl_tcam_mr_ruleset_group_id,
.rule_priv_size = mlxsw_sp_acl_tcam_mr_rule_priv_size, .rule_priv_size = sizeof(struct mlxsw_sp_acl_tcam_mr_rule),
.rule_add = mlxsw_sp_acl_tcam_mr_rule_add, .rule_add = mlxsw_sp_acl_tcam_mr_rule_add,
.rule_del = mlxsw_sp_acl_tcam_mr_rule_del, .rule_del = mlxsw_sp_acl_tcam_mr_rule_del,
.rule_action_replace = mlxsw_sp_acl_tcam_mr_rule_action_replace, .rule_action_replace = mlxsw_sp_acl_tcam_mr_rule_action_replace,
......
...@@ -43,7 +43,7 @@ struct mlxsw_sp_acl_profile_ops { ...@@ -43,7 +43,7 @@ struct mlxsw_sp_acl_profile_ops {
struct mlxsw_sp_port *mlxsw_sp_port, struct mlxsw_sp_port *mlxsw_sp_port,
bool ingress); bool ingress);
u16 (*ruleset_group_id)(void *ruleset_priv); u16 (*ruleset_group_id)(void *ruleset_priv);
size_t (*rule_priv_size)(struct mlxsw_sp *mlxsw_sp); size_t rule_priv_size;
int (*rule_add)(struct mlxsw_sp *mlxsw_sp, int (*rule_add)(struct mlxsw_sp *mlxsw_sp,
void *ruleset_priv, void *rule_priv, void *ruleset_priv, void *rule_priv,
struct mlxsw_sp_acl_rule_info *rulei); struct mlxsw_sp_acl_rule_info *rulei);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment