On Tue, 1 Feb 2011, Jozsef Kadlecsik wrote:

> On Tue, 1 Feb 2011, Jozsef Kadlecsik wrote:
> 
> > On Tue, 1 Feb 2011, Patrick McHardy wrote:
> > 
> > > Am 31.01.2011 23:52, schrieb Jozsef Kadlecsik:
> > > > +static int
> > > > +call_ad(struct sk_buff *skb, struct ip_set *set,
> > > > +	struct nlattr *tb[], enum ipset_adt adt,
> > > > +	u32 flags, bool use_lineno)
> > > > +{
> > > > +	int ret, retried = 0;
> > > > +	u32 lineno = 0;
> > > > +	bool eexist = flags & IPSET_FLAG_EXIST;
> > > > +
> > > > +	do {
> > > > +		write_lock_bh(&set->lock);
> > > > +		ret = set->variant->uadt(set, tb, adt, &lineno, flags);
> > > > +		write_unlock_bh(&set->lock);
> > > > +	} while (ret == -EAGAIN &&
> > > > +		 set->variant->resize &&
> > > > +		 (ret = set->variant->resize(set, retried++)) == 0);
> > > > +
> > > > +	if (!ret || (ret == -IPSET_ERR_EXIST && eexist))
> > > > +		return 0;
> > > > +	if (lineno && use_lineno) {
> > > > +		/* Error in restore/batch mode: send back lineno */
> > > > +		struct nlmsghdr *nlh = nlmsg_hdr(skb);
> > > > +		int min_len = NLMSG_SPACE(sizeof(struct nfgenmsg));
> > > > +		struct nlattr *cda[IPSET_ATTR_CMD_MAX+1];
> > > > +		struct nlattr *cmdattr = (void *)nlh + min_len;
> > > > +		u32 *errline;
> > > > +
> > > > +		nla_parse(cda, IPSET_ATTR_CMD_MAX,
> > > > +			  cmdattr, nlh->nlmsg_len - min_len,
> > > > +			  ip_set_adt_policy);
> > > > +
> > > > +		errline = nla_data(cda[IPSET_ATTR_LINENO]);
> > > > +
> > > > +		*errline = lineno;
> > > 
> > > This is still not correct. I didn't mean to remove the const attributes
> > > (the message is still considered const by the higher layers, the netlink
> > > functions just cast this away). You're modifying the received message,
> > > I don't see how this can be useful to userspace.
> > 
> > I can't find where the message is considered const in netlink/nfnetlink.
> > It seems to be freely writable via skb.
> >  
> > > I guess you're relying on that the original message is appended to a
> > > nlmsgerr message. That doesn't seem right though, if you want to return
> > > something to userspace, you should construct a new message.
> > 
> > The message we are processing here carried multiple commands (each having 
> > an attribute with the line number of the given command) and one failed 
> > from some reason. We have to notify the userspace which command, at what 
> > line failed. For this reason the multi-command messages have got an 
> > attribute, which can be filled out with the line number - that happens 
> > here. The attribute is already there, the message is not enlarged, just
> > the empty value is overwritten with the proper value.
> > 
> > The line number reporting works this way, tested in the testsuite too.
> > 
> > If I had to construct a completely new message and sent it, that'd be more 
> > or less the duplication of netlink_ack. Additionally I had to suppress 
> > netlink from sending an errmsg/ack too.
> 
> Hm, if I lie -EINTR to netlink, then I can construct and send the error 
> message manually and keep NLM_F_ACK at the same time. What do you think?
> Please have a look at the attached patch.

Oops, mistypeing fixed, here follow the hopefully good version.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary