[ovs-dev] [PATCH 2/2] datapath: Move over-MTU checking into vport_send().
Jesse Gross
jesse at nicira.com
Tue Jul 13 21:11:00 UTC 2010
On Tue, Jul 13, 2010 at 11:43 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Mon, Jul 12, 2010 at 02:31:17PM -0700, Jesse Gross wrote:
> > @@ -1276,6 +1277,17 @@ vport_receive(struct vport *vport, struct sk_buff
> *skb)
> > dp_process_received_packet(dp_port, skb);
> > }
> >
> > +static inline unsigned
> > +packet_length(const struct sk_buff *skb)
>
> Kernel style usually keeps the return type and the arguments on a single
> line. I guess we're not doing that in vport.c. Perhaps we should.
>
There are a bunch of places where we do this. When I get a chance, I'll
just make a pass through all of our kernel code and make them consistent.
>
> > /**
> > * vport_send - send a packet on a device
> > *
> > @@ -1289,24 +1301,26 @@ int
> > vport_send(struct vport *vport, struct sk_buff *skb)
> > {
> > int *loop_count;
> > + int mtu;
> > int sent;
> >
> > loop_count = &per_cpu_ptr(vport_loop_counter,
> get_cpu())->count[!!in_interrupt()];
> > (*loop_count)++;
> >
> > - if (likely((*loop_count) <= VPORT_MAX_LOOPS)) {
> > - sent = vport->ops->send(vport, skb);
> > - } else {
> > + if (unlikely((*loop_count) > VPORT_MAX_LOOPS)) {
>
> I think that the extra () around *loop_count makes this harder to read.
> Mind dropping them?
>
Yeah, I probably did it because I just finished typing it for the line
above.
>
> > printk(KERN_WARNING "%s: dropping packet that has looped
> more than %d times\n",
> > dp_name(vport_get_dp_port(vport)->dp),
> VPORT_MAX_LOOPS);
> > + goto error;
> > + }
>
> Shouldn't we rate-limit that message?
>
Yeah, I added that.
>
> (I should have pointed both of those out for patch 1/2.)
>
> >
> > - sent = 0;
> > - kfree_skb(skb);
> > - vport_record_error(vport, VPORT_E_TX_DROPPED);
> > + mtu = vport_get_mtu(vport);
> > + if (unlikely(packet_length(skb) > mtu && !skb_is_gso(skb))) {
> > + printk(KERN_WARNING "%s: dropped over-mtu packet: %d >
> %d\n",
> > + dp_name(vport_get_dp_port(vport)->dp),
> packet_length(skb), mtu);
> > + goto error;
> > }
>
> It seems like we should rate-limit this one too. I wonder why we don't.
>
Added it here too.
I pushed these two patches.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100713/d606af7d/attachment-0003.html>
More information about the dev
mailing list