On 8/6/19 1:06 PM, Pablo Neira Ayuso wrote:
> On Tue, Aug 06, 2019 at 12:29:45PM +0200, Fernando Fernandez Mancera wrote:
>> I have been thinking of a way to update a quota object. i.e raise or lower 
>> the
>> quota limit of an existing object. I think it would be ideal to implement the
>> operations of updating objects in the API in a generic way.
>>
>> Therefore, we could easily give update support to each object type by adding 
>> an
>> update operation in the "nft_object_ops" struct. This is a conceptual patch 
>> so
>> it does not work.
>>
>> Signed-off-by: Fernando Fernandez Mancera <ffmanc...@riseup.net>
>> ---
>>  include/net/netfilter/nf_tables.h        |  4 ++++
>>  include/uapi/linux/netfilter/nf_tables.h |  2 ++
>>  net/netfilter/nf_tables_api.c            | 22 ++++++++++++++++++++++
>>  3 files changed, 28 insertions(+)
>>
>> diff --git a/include/net/netfilter/nf_tables.h 
>> b/include/net/netfilter/nf_tables.h
>> index 9b624566b82d..bd1e6c19d23f 100644
>> --- a/include/net/netfilter/nf_tables.h
>> +++ b/include/net/netfilter/nf_tables.h
>> @@ -1101,6 +1101,7 @@ struct nft_object_type {
>>   *  @eval: stateful object evaluation function
>>   *  @size: stateful object size
>>   *  @init: initialize object from netlink attributes
>> + *  @update: update object from netlink attributes
>>   *  @destroy: release existing stateful object
>>   *  @dump: netlink dump stateful object
>>   */
>> @@ -1112,6 +1113,9 @@ struct nft_object_ops {
>>      int                             (*init)(const struct nft_ctx *ctx,
>>                                              const struct nlattr *const tb[],
>>                                              struct nft_object *obj);
>> +    int                             (*update)(const struct nft_ctx *ctx,
>> +                                              const struct nlattr *const 
>> tb[],
>> +                                              struct nft_object *obj);
>>      void                            (*destroy)(const struct nft_ctx *ctx,
>>                                                 struct nft_object *obj);
>>      int                             (*dump)(struct sk_buff *skb,
>> diff --git a/include/uapi/linux/netfilter/nf_tables.h 
>> b/include/uapi/linux/netfilter/nf_tables.h
>> index 82abaa183fc3..8b0a012e9177 100644
>> --- a/include/uapi/linux/netfilter/nf_tables.h
>> +++ b/include/uapi/linux/netfilter/nf_tables.h
>> @@ -92,6 +92,7 @@ enum nft_verdicts {
>>   * @NFT_MSG_NEWOBJ: create a stateful object (enum nft_obj_attributes)
>>   * @NFT_MSG_GETOBJ: get a stateful object (enum nft_obj_attributes)
>>   * @NFT_MSG_DELOBJ: delete a stateful object (enum nft_obj_attributes)
>> + * @NFT_MSG_UPDOBJ: update a stateful object (enum nft_obj_attributes)
>>   * @NFT_MSG_GETOBJ_RESET: get and reset a stateful object (enum 
>> nft_obj_attributes)
>>   * @NFT_MSG_NEWFLOWTABLE: add new flow table (enum nft_flowtable_attributes)
>>   * @NFT_MSG_GETFLOWTABLE: get flow table (enum nft_flowtable_attributes)
>> @@ -119,6 +120,7 @@ enum nf_tables_msg_types {
>>      NFT_MSG_NEWOBJ,
>>      NFT_MSG_GETOBJ,
>>      NFT_MSG_DELOBJ,
>> +    NFT_MSG_UPDOBJ,
> 
> No need for this new message type, see below.
> 
>>      NFT_MSG_GETOBJ_RESET,
>>      NFT_MSG_NEWFLOWTABLE,
>>      NFT_MSG_GETFLOWTABLE,
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index 605a7cfe7ca7..c7267f418808 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -5420,6 +5420,16 @@ static void nft_obj_destroy(const struct nft_ctx 
>> *ctx, struct nft_object *obj)
>>      kfree(obj);
>>  }
>>  
>> +static int nf_tables_updobj(struct net *net, struct sock *nlsk,
>> +                        struct sk_buff *skb, const struct nlmsghdr *nlh,
>> +                        const struct nlattr * const nla[],
>> +                        struct netlink_ext_ack *extack)
>> +{
>> +    /* Placeholder function, here we would need to check if the object
>> +     * exists. Then init the context and update the object.*/
>> +    return 1;
> 
> Use the existing nf_tables_newobj(), if NLM_F_EXCL is not set on and
> the object exists, then this is an update.
> 

I agree on that. But I think that if we use the NFT_MSG_NEWOBJ there
will be some issues in the commit and the abort phase. That is why I
think "NFT_MSG_UPDOBJ" would be needed.

Thanks!

Reply via email to