Re: [RESEND 3/5] [NET]: Protocol Independant Policy Routing Rules Framework

[Thomas Graf <tgraf@suug.ch>]

* Patrick McHardy <kaber@trash.net> 2006-07-28 01:30
> > +int fib_rules_lookup(struct fib_rules_ops *ops, struct flowi *fl,
> > +		     int flags, struct fib_lookup_arg *arg)
> > +{
> > +	struct fib_rule *rule;
> > +	int err;
> > +
> > +	rcu_read_lock();
> > +
> > +	list_for_each_entry(rule, ops->rules_list, list) {
> > +		if (rule->ifname[0] && (rule->ifindex != fl->iif))
> > +			continue;
> 
> ifindex may be unset even if ifname is set (in case the interface
> does not exist yet). In that case it will match falsely on
> locally generated packets.

Then rule->ifindex would be -1 and it shouldn't match but I
changed it, it makes more sense.

> > +static void notify_rule_change(int event, struct fib_rule *rule,
> > +			       struct fib_rules_ops *ops)
> > +{
> > +	int size = nlmsg_total_size(sizeof(struct fib_rule_hdr) + 128);
> > +	struct sk_buff *skb = alloc_skb(size, GFP_KERNEL);
> > +
> > +	if (skb == NULL)
> > +		netlink_set_err(rtnl, 0, RTNLGRP_IPV4_RULE, ENOBUFS);
> > +	else if (fib_nl_fill_rule(skb, rule, 0, 0, event, 0, ops) < 0) {
> > +		kfree_skb(skb);
> > +		netlink_set_err(rtnl, 0, RTNLGRP_IPV4_RULE, EINVAL);
> > +	} else
> > +		netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV4_RULE, GFP_KERNEL);
> > +}
> 
> Shouldn't different families use different groups? Userspace
> might (rightfully, I think) expect not to see anything but
> IPv4 rules on RTNLGRP_IPV4_RULE.

Right, I've added ops->nlgroup to fix this. Naturally I also
fixed all the other issues you brought up, I have the feeling
that there are more bugs, will look at the code again with
some distance in a few days.