[ovs-dev,ovn] pinctrl: fix IP buffering with connection-tracking
diff mbox series

Message ID 0f0989dedc00b85af688df35ab0d444c09093a5c.1581611906.git.lorenzo.bianconi@redhat.com
State New
Headers show
Series
  • [ovs-dev,ovn] pinctrl: fix IP buffering with connection-tracking
Related show

Commit Message

Lorenzo Bianconi Feb. 13, 2020, 4:49 p.m. UTC
Whenever we need to reinject an IP packet buffered during L2 address
resolution we need to preserve ovs ofport in order to let ovs
connection tracking to properly SNAT/DNAT the packet.
Do not overwrite the MFF_IN_PORT in consider_port_binding routine

Signed-off-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/physical.c | 1 -
 controller/pinctrl.c  | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

0-day Robot Feb. 13, 2020, 4:57 p.m. UTC | #1
Bleep bloop.  Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Numan Siddique <numans@ovn.org>
Lines checked: 62, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Ben Pfaff Feb. 13, 2020, 7:12 p.m. UTC | #2
On Thu, Feb 13, 2020 at 05:49:37PM +0100, Lorenzo Bianconi wrote:
> Whenever we need to reinject an IP packet buffered during L2 address
> resolution we need to preserve ovs ofport in order to let ovs
> connection tracking to properly SNAT/DNAT the packet.
> Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

The sign-offs are a bit odd here.  Should one be an Acked-by?
Numan Siddique Feb. 13, 2020, 7:49 p.m. UTC | #3
On Fri, Feb 14, 2020 at 12:43 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Thu, Feb 13, 2020 at 05:49:37PM +0100, Lorenzo Bianconi wrote:
> > Whenever we need to reinject an IP packet buffered during L2 address
> > resolution we need to preserve ovs ofport in order to let ovs
> > connection tracking to properly SNAT/DNAT the packet.
> > Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
> The sign-offs are a bit odd here.  Should one be an Acked-by?

Hi Ben,

I had suggested him this approach to solve this problem. Probably he
can use  the tag - "Suggested-by: Numan Siddique <numans@ovn.org>"

Thanks
Numan




> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Lorenzo Bianconi Feb. 13, 2020, 11:38 p.m. UTC | #4
>
> On Fri, Feb 14, 2020 at 12:43 AM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Thu, Feb 13, 2020 at 05:49:37PM +0100, Lorenzo Bianconi wrote:
> > > Whenever we need to reinject an IP packet buffered during L2 address
> > > resolution we need to preserve ovs ofport in order to let ovs
> > > connection tracking to properly SNAT/DNAT the packet.
> > > Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> > >
> > > Signed-off-by: Numan Siddique <numans@ovn.org>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> >
> > The sign-offs are a bit odd here.  Should one be an Acked-by?
>
> Hi Ben,
>
> I had suggested him this approach to solve this problem. Probably he
> can use  the tag - "Suggested-by: Numan Siddique <numans@ovn.org>"
>
> Thanks
> Numan

yes, sorry for the noise Ben.
Do I need to resubmit or is it ok to fix this patch?

Regards,
Lorenzo

>
>
>
>
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
Ben Pfaff Feb. 13, 2020, 11:41 p.m. UTC | #5
On Fri, Feb 14, 2020 at 12:38:54AM +0100, Lorenzo Bianconi wrote:
> >
> > On Fri, Feb 14, 2020 at 12:43 AM Ben Pfaff <blp@ovn.org> wrote:
> > >
> > > On Thu, Feb 13, 2020 at 05:49:37PM +0100, Lorenzo Bianconi wrote:
> > > > Whenever we need to reinject an IP packet buffered during L2 address
> > > > resolution we need to preserve ovs ofport in order to let ovs
> > > > connection tracking to properly SNAT/DNAT the packet.
> > > > Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> > > >
> > > > Signed-off-by: Numan Siddique <numans@ovn.org>
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> > >
> > > The sign-offs are a bit odd here.  Should one be an Acked-by?
> >
> > Hi Ben,
> >
> > I had suggested him this approach to solve this problem. Probably he
> > can use  the tag - "Suggested-by: Numan Siddique <numans@ovn.org>"
> >
> > Thanks
> > Numan
> 
> yes, sorry for the noise Ben.
> Do I need to resubmit or is it ok to fix this patch?

No need, I can fix that bit.
Dumitru Ceara Feb. 19, 2020, 8:31 a.m. UTC | #6
On 2/13/20 5:49 PM, Lorenzo Bianconi wrote:
> Whenever we need to reinject an IP packet buffered during L2 address
> resolution we need to preserve ovs ofport in order to let ovs
> connection tracking to properly SNAT/DNAT the packet.
> Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Hi Lorenzo,

The code looks good to me.

I couldn't find any side effects due to not resetting MFF_IN_PORT when
entering the router pipeline but I'm wondering if it's possible to add a
test case for the issue you're fixing.

This way, if we did miss a case when the now non-zero in_port is causing
problems in the router pipeline we at least know that zeroing it out
will trigger the new test to fail.

Thanks,
Dumitru

> ---
>  controller/physical.c | 1 -
>  controller/pinctrl.c  | 5 ++++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/controller/physical.c b/controller/physical.c
> index a7edaddac..144aeb7bd 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -905,7 +905,6 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>          for (int i = 0; i < MFF_N_LOG_REGS; i++) {
>              put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
>          }
> -        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
>          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
>          clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
>          ofpacts_p->header = clone;
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 09b0f7bf7..d06915a65 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -559,6 +559,7 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
>  
>  struct buffer_info {
>      struct ofpbuf ofpacts;
> +    ofp_port_t ofp_port;
>      struct dp_packet *p;
>  };
>  
> @@ -629,6 +630,8 @@ buffered_push_packet(struct buffered_packets *bp,
>      ofpbuf_init(&bi->ofpacts, 4096);
>  
>      reload_metadata(&bi->ofpacts, md);
> +    bi->ofp_port = md->flow.in_port.ofp_port;
> +
>      struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&bi->ofpacts);
>      resubmit->in_port = OFPP_CONTROLLER;
>      resubmit->table_id = OFTABLE_REMOTE_OUTPUT;
> @@ -663,7 +666,7 @@ buffered_send_packets(struct rconn *swconn, struct buffered_packets *bp,
>              .ofpacts = bi->ofpacts.data,
>              .ofpacts_len = bi->ofpacts.size,
>          };
> -        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
> +        match_set_in_port(&po.flow_metadata, bi->ofp_port);
>          queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>  
>          ofpbuf_uninit(&bi->ofpacts);
>
Lorenzo Bianconi Feb. 19, 2020, 10:02 a.m. UTC | #7
On Feb 19, Dumitru Ceara wrote:
> On 2/13/20 5:49 PM, Lorenzo Bianconi wrote:
> > Whenever we need to reinject an IP packet buffered during L2 address
> > resolution we need to preserve ovs ofport in order to let ovs
> > connection tracking to properly SNAT/DNAT the packet.
> > Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> > 
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> 
> Hi Lorenzo,

Hi Dumitru,

thanks for the review :)

> 
> The code looks good to me.
> 
> I couldn't find any side effects due to not resetting MFF_IN_PORT when
> entering the router pipeline but I'm wondering if it's possible to add a
> test case for the issue you're fixing.

I guess test-cases are already in system-ovn.at:

- ovn -- DNAT and SNAT on distributed router - N/S
- ovn -- DNAT and SNAT on distributed router - N/S - IPv6
- ovn -- DNAT and SNAT on distributed router - E/W
- ovn -- DNAT and SNAT on distributed router - E/W - IPv6

but they actually run a single device.

Regards,
Lorenzo

> 
> This way, if we did miss a case when the now non-zero in_port is causing
> problems in the router pipeline we at least know that zeroing it out
> will trigger the new test to fail.
> 
> Thanks,
> Dumitru
> 
> > ---
> >  controller/physical.c | 1 -
> >  controller/pinctrl.c  | 5 ++++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/controller/physical.c b/controller/physical.c
> > index a7edaddac..144aeb7bd 100644
> > --- a/controller/physical.c
> > +++ b/controller/physical.c
> > @@ -905,7 +905,6 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >          for (int i = 0; i < MFF_N_LOG_REGS; i++) {
> >              put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
> >          }
> > -        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
> >          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> >          clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
> >          ofpacts_p->header = clone;
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index 09b0f7bf7..d06915a65 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -559,6 +559,7 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
> >  
> >  struct buffer_info {
> >      struct ofpbuf ofpacts;
> > +    ofp_port_t ofp_port;
> >      struct dp_packet *p;
> >  };
> >  
> > @@ -629,6 +630,8 @@ buffered_push_packet(struct buffered_packets *bp,
> >      ofpbuf_init(&bi->ofpacts, 4096);
> >  
> >      reload_metadata(&bi->ofpacts, md);
> > +    bi->ofp_port = md->flow.in_port.ofp_port;
> > +
> >      struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&bi->ofpacts);
> >      resubmit->in_port = OFPP_CONTROLLER;
> >      resubmit->table_id = OFTABLE_REMOTE_OUTPUT;
> > @@ -663,7 +666,7 @@ buffered_send_packets(struct rconn *swconn, struct buffered_packets *bp,
> >              .ofpacts = bi->ofpacts.data,
> >              .ofpacts_len = bi->ofpacts.size,
> >          };
> > -        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
> > +        match_set_in_port(&po.flow_metadata, bi->ofp_port);
> >          queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
> >  
> >          ofpbuf_uninit(&bi->ofpacts);
> > 
>
Dumitru Ceara Feb. 19, 2020, 10:59 a.m. UTC | #8
On 2/19/20 11:02 AM, Lorenzo Bianconi wrote:
> On Feb 19, Dumitru Ceara wrote:
>> On 2/13/20 5:49 PM, Lorenzo Bianconi wrote:
>>> Whenever we need to reinject an IP packet buffered during L2 address
>>> resolution we need to preserve ovs ofport in order to let ovs
>>> connection tracking to properly SNAT/DNAT the packet.
>>> Do not overwrite the MFF_IN_PORT in consider_port_binding routine
>>>
>>> Signed-off-by: Numan Siddique <numans@ovn.org>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>>
>> Hi Lorenzo,
> 
> Hi Dumitru,
> 
> thanks for the review :)
> 
>>
>> The code looks good to me.
>>
>> I couldn't find any side effects due to not resetting MFF_IN_PORT when
>> entering the router pipeline but I'm wondering if it's possible to add a
>> test case for the issue you're fixing.
> 
> I guess test-cases are already in system-ovn.at:
> 
> - ovn -- DNAT and SNAT on distributed router - N/S
> - ovn -- DNAT and SNAT on distributed router - N/S - IPv6
> - ovn -- DNAT and SNAT on distributed router - E/W
> - ovn -- DNAT and SNAT on distributed router - E/W - IPv6
> 
> but they actually run a single device.
> 
> Regards,
> Lorenzo

I see, thanks.

Acked-by: Dumitru Ceara <dceara@redhat.com>

> 
>>
>> This way, if we did miss a case when the now non-zero in_port is causing
>> problems in the router pipeline we at least know that zeroing it out
>> will trigger the new test to fail.
>>
>> Thanks,
>> Dumitru
>>
>>> ---
>>>  controller/physical.c | 1 -
>>>  controller/pinctrl.c  | 5 ++++-
>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/controller/physical.c b/controller/physical.c
>>> index a7edaddac..144aeb7bd 100644
>>> --- a/controller/physical.c
>>> +++ b/controller/physical.c
>>> @@ -905,7 +905,6 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>>>          for (int i = 0; i < MFF_N_LOG_REGS; i++) {
>>>              put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
>>>          }
>>> -        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
>>>          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
>>>          clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
>>>          ofpacts_p->header = clone;
>>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>>> index 09b0f7bf7..d06915a65 100644
>>> --- a/controller/pinctrl.c
>>> +++ b/controller/pinctrl.c
>>> @@ -559,6 +559,7 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
>>>  
>>>  struct buffer_info {
>>>      struct ofpbuf ofpacts;
>>> +    ofp_port_t ofp_port;
>>>      struct dp_packet *p;
>>>  };
>>>  
>>> @@ -629,6 +630,8 @@ buffered_push_packet(struct buffered_packets *bp,
>>>      ofpbuf_init(&bi->ofpacts, 4096);
>>>  
>>>      reload_metadata(&bi->ofpacts, md);
>>> +    bi->ofp_port = md->flow.in_port.ofp_port;
>>> +
>>>      struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&bi->ofpacts);
>>>      resubmit->in_port = OFPP_CONTROLLER;
>>>      resubmit->table_id = OFTABLE_REMOTE_OUTPUT;
>>> @@ -663,7 +666,7 @@ buffered_send_packets(struct rconn *swconn, struct buffered_packets *bp,
>>>              .ofpacts = bi->ofpacts.data,
>>>              .ofpacts_len = bi->ofpacts.size,
>>>          };
>>> -        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>>> +        match_set_in_port(&po.flow_metadata, bi->ofp_port);
>>>          queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>>>  
>>>          ofpbuf_uninit(&bi->ofpacts);
>>>
>>
Numan Siddique Feb. 19, 2020, 12:41 p.m. UTC | #9
On Wed, Feb 19, 2020 at 4:29 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 2/19/20 11:02 AM, Lorenzo Bianconi wrote:
> > On Feb 19, Dumitru Ceara wrote:
> >> On 2/13/20 5:49 PM, Lorenzo Bianconi wrote:
> >>> Whenever we need to reinject an IP packet buffered during L2 address
> >>> resolution we need to preserve ovs ofport in order to let ovs
> >>> connection tracking to properly SNAT/DNAT the packet.
> >>> Do not overwrite the MFF_IN_PORT in consider_port_binding routine
> >>>
> >>> Signed-off-by: Numan Siddique <numans@ovn.org>
> >>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> >>
> >> Hi Lorenzo,
> >
> > Hi Dumitru,
> >
> > thanks for the review :)
> >
> >>
> >> The code looks good to me.
> >>
> >> I couldn't find any side effects due to not resetting MFF_IN_PORT when
> >> entering the router pipeline but I'm wondering if it's possible to add a
> >> test case for the issue you're fixing.
> >
> > I guess test-cases are already in system-ovn.at:
> >
> > - ovn -- DNAT and SNAT on distributed router - N/S
> > - ovn -- DNAT and SNAT on distributed router - N/S - IPv6
> > - ovn -- DNAT and SNAT on distributed router - E/W
> > - ovn -- DNAT and SNAT on distributed router - E/W - IPv6
> >
> > but they actually run a single device.
> >
> > Regards,
> > Lorenzo
>
> I see, thanks.
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks for the review Dumitru.

I applied this patch to master and branch-20.03

Thanks
Numan

>
> >
> >>
> >> This way, if we did miss a case when the now non-zero in_port is causing
> >> problems in the router pipeline we at least know that zeroing it out
> >> will trigger the new test to fail.
> >>
> >> Thanks,
> >> Dumitru
> >>
> >>> ---
> >>>  controller/physical.c | 1 -
> >>>  controller/pinctrl.c  | 5 ++++-
> >>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/controller/physical.c b/controller/physical.c
> >>> index a7edaddac..144aeb7bd 100644
> >>> --- a/controller/physical.c
> >>> +++ b/controller/physical.c
> >>> @@ -905,7 +905,6 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
> >>>          for (int i = 0; i < MFF_N_LOG_REGS; i++) {
> >>>              put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
> >>>          }
> >>> -        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
> >>>          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> >>>          clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
> >>>          ofpacts_p->header = clone;
> >>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> >>> index 09b0f7bf7..d06915a65 100644
> >>> --- a/controller/pinctrl.c
> >>> +++ b/controller/pinctrl.c
> >>> @@ -559,6 +559,7 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
> >>>
> >>>  struct buffer_info {
> >>>      struct ofpbuf ofpacts;
> >>> +    ofp_port_t ofp_port;
> >>>      struct dp_packet *p;
> >>>  };
> >>>
> >>> @@ -629,6 +630,8 @@ buffered_push_packet(struct buffered_packets *bp,
> >>>      ofpbuf_init(&bi->ofpacts, 4096);
> >>>
> >>>      reload_metadata(&bi->ofpacts, md);
> >>> +    bi->ofp_port = md->flow.in_port.ofp_port;
> >>> +
> >>>      struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&bi->ofpacts);
> >>>      resubmit->in_port = OFPP_CONTROLLER;
> >>>      resubmit->table_id = OFTABLE_REMOTE_OUTPUT;
> >>> @@ -663,7 +666,7 @@ buffered_send_packets(struct rconn *swconn, struct buffered_packets *bp,
> >>>              .ofpacts = bi->ofpacts.data,
> >>>              .ofpacts_len = bi->ofpacts.size,
> >>>          };
> >>> -        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
> >>> +        match_set_in_port(&po.flow_metadata, bi->ofp_port);
> >>>          queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
> >>>
> >>>          ofpbuf_uninit(&bi->ofpacts);
> >>>
> >>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch
diff mbox series

diff --git a/controller/physical.c b/controller/physical.c
index a7edaddac..144aeb7bd 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -905,7 +905,6 @@  consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
         for (int i = 0; i < MFF_N_LOG_REGS; i++) {
             put_load(0, MFF_LOG_REG0 + i, 0, 32, ofpacts_p);
         }
-        put_load(0, MFF_IN_PORT, 0, 16, ofpacts_p);
         put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
         clone = ofpbuf_at_assert(ofpacts_p, clone_ofs, sizeof *clone);
         ofpacts_p->header = clone;
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 09b0f7bf7..d06915a65 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -559,6 +559,7 @@  set_actions_and_enqueue_msg(struct rconn *swconn,
 
 struct buffer_info {
     struct ofpbuf ofpacts;
+    ofp_port_t ofp_port;
     struct dp_packet *p;
 };
 
@@ -629,6 +630,8 @@  buffered_push_packet(struct buffered_packets *bp,
     ofpbuf_init(&bi->ofpacts, 4096);
 
     reload_metadata(&bi->ofpacts, md);
+    bi->ofp_port = md->flow.in_port.ofp_port;
+
     struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&bi->ofpacts);
     resubmit->in_port = OFPP_CONTROLLER;
     resubmit->table_id = OFTABLE_REMOTE_OUTPUT;
@@ -663,7 +666,7 @@  buffered_send_packets(struct rconn *swconn, struct buffered_packets *bp,
             .ofpacts = bi->ofpacts.data,
             .ofpacts_len = bi->ofpacts.size,
         };
-        match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
+        match_set_in_port(&po.flow_metadata, bi->ofp_port);
         queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
 
         ofpbuf_uninit(&bi->ofpacts);