ng_patch node

[Alexander Motin]

Maxim Ignatenko wrote:
> I've written netgraph node able to modify arbitrary (8|16|32)-bit
> unsigned integer in passing packets. Node applies one of =,+,-,&,| and
> ^ operations to number at given offset.
> Modification applied to each packet received on "in" hook. If "out"
> hook is connected - resulting packets passed on it, otherwise -
> returned back on "in" (for more easy use with ng_ipfw). Packets
> received on "out" hook passed on "in" unmodified.
> Node supports two control messages: "getconfig" and "setconfig".
> Configuration represented in next structure:
> struct ng_patch_config {
>        uint32_t        value; /* argument passed to requested operation */
>        uint32_t        offset; /* offset in bytes */
>        uint32_t        length; /* 1,2 or 4 bytes */
>        uint32_t        mode; /* operation code: 1 - "=", 2 - "+", 3 -
> "-", 4 - "&", 5 - "|", 6 - "^" */
> };
> Same names used in ASCII representation.
> 
> I wanted to make ipfw able to modify TTL and ToS fields in IP packets,
> but after some generalization idea looked like described above.
> 
> Next patch made against 8-STABLE r200201

Just few stones into your garden:

> +                       if (((struct ng_patch_config *)msg->data)->offset < 0)
> +                               error = EINVAL;

As I see, offset field is unsigned there.

> +                                       case 4:
> +                                               *((uint32_t *)dst) +=
> priv->value4;

I think such dereference may crash archs with strong alignment
requirements. m_copydata/m_copyback could do it possibly slower, but
safer and wouldn't require m_pullup.

Also result of such multi-byte operations is endian-dependent. I would
be nice to do hton/ntoh somewhere.

Also, what's about checksums?

-- 
Alexander Motin