Pablo Neira Ayuso <pa...@netfilter.org> wrote:
> > --- a/net/bridge/netfilter/ebt_among.c
> > +++ b/net/bridge/netfilter/ebt_among.c
> > @@ -187,17 +187,17 @@ static int ebt_among_mt_check(const struct 
> > xt_mtchk_param *par)
> >     expected_length += ebt_mac_wormhash_size(wh_src);
> >  
> >     if (em->match_size != EBT_ALIGN(expected_length)) {
> > -           pr_info("wrong size: %d against expected %d, rounded to %zd\n",
> > -                   em->match_size, expected_length,
> > -                   EBT_ALIGN(expected_length));
> > +           pr_info_ratelimited("wrong size: %d against expected %d, 
> > rounded to %zd\n",
> 
> Shouldn't all these be pr_err_ratelimited instead?

Don't know.

This could even be pr_debug actually since this message is
useless unless you're doing ebtables development work.

> Probably this is a good chance to homogeneize all error reporting in
> xtables.

Yes.

> >     if (wh_dst && (err = ebt_mac_wormhash_check_integrity(wh_dst))) {
> > -           pr_info("dst integrity fail: %x\n", -err);
> > +           pr_info_ratelimited("dst integrity fail: %x\n", -err);
> >             return -EINVAL;
> >     }
> >     if (wh_src && (err = ebt_mac_wormhash_check_integrity(wh_src))) {
> > -           pr_info("src integrity fail: %x\n", -err);
> > +           pr_info_ratelimited("src integrity fail: %x\n", -err);
> >             return -EINVAL;

Same for these two, I'll convert all to pr_debug instead.

> >     if (info->queues_total == 0) {
> > -           pr_err("NFQUEUE: number of total queues is 0\n");
>                         ^^^^^^^^
> 
> We can probably add this all over the place in the same go?
 
>         #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Yes.

> >     if (index == IPSET_INVALID_ID) {
> > -           pr_warn("Cannot find set identified by id %u to match\n",
> > -                   info->match_set.index);
> > +           pr_warn_ratelimited("Cannot find set identified by id %u to 
> > match\n",
> > +                               info->match_set.index);
> 
> Use pr_err_ratelimited instead?

I think we should settle on a single pr_foo, i suggest
pr_info(_ratelimited).

This is not an error condition, we only have these
printks because we can't return a proper error to userspace.

If this was netlink, it would be converted to extack instead...
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to